Re: [RFC PATCH v2 10/12] powerpc/ptrace: create ptrace_get_debugreg()

2019-07-02 Thread Ravi Bangoria



On 6/28/19 9:17 PM, Christophe Leroy wrote:
> Create ptrace_get_debugreg() to handle PTRACE_GET_DEBUGREG and
> reduce ifdef mess
> 
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/kernel/ptrace/ptrace-adv.c   |  9 +
>  arch/powerpc/kernel/ptrace/ptrace-decl.h  |  2 ++
>  arch/powerpc/kernel/ptrace/ptrace-noadv.c | 13 +
>  arch/powerpc/kernel/ptrace/ptrace.c   | 18 ++
>  4 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c 
> b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> index 86e71fa6c5c8..dcc765940344 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> @@ -83,6 +83,15 @@ void user_disable_single_step(struct task_struct *task)
>   clear_tsk_thread_flag(task, TIF_SINGLESTEP);
>  }
>  
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp)
> +{
> + /* We only support one DABR and no IABRS at the moment */

No DABR / IABR in ptrace-adv.c

> + if (addr > 0)
> + return -EINVAL;
> + return put_user(child->thread.debug.dac1, datalp);
> +}
> +
>  int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, 
> unsigned long data)
>  {
>   /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h 
> b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> index bdba09a87aea..4b4b6a1d508a 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
> +++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> @@ -176,6 +176,8 @@ int tm_cgpr32_set(struct task_struct *target, const 
> struct user_regset *regset,
>  extern const struct user_regset_view user_ppc_native_view;
>  
>  /* ptrace-(no)adv */
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp);
>  int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, 
> unsigned long data);
>  long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint 
> *bp_info);
>  long ppc_del_hwdebug(struct task_struct *child, long data);
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c 
> b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> index 7db330c94538..985cca136f85 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> @@ -64,6 +64,19 @@ void user_disable_single_step(struct task_struct *task)
>   clear_tsk_thread_flag(task, TIF_SINGLESTEP);
>  }
>  
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp)
> +{
> + unsigned long dabr_fake;
> +
> + /* We only support one DABR and no IABRS at the moment */
> + if (addr > 0)
> + return -EINVAL;
> + dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> +  (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + return put_user(dabr_fake, datalp);
> +}
> +
>  int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, 
> unsigned long data)
>  {
>  #ifdef CONFIG_HAVE_HW_BREAKPOINT
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c 
> b/arch/powerpc/kernel/ptrace/ptrace.c
> index 377e0e541d5f..e789afae6f56 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -211,23 +211,9 @@ long arch_ptrace(struct task_struct *child, long request,
>   break;
>   }
>  
> - case PTRACE_GET_DEBUGREG: {
> -#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> - unsigned long dabr_fake;
> -#endif
> - ret = -EINVAL;
> - /* We only support one DABR and no IABRS at the moment */
> - if (addr > 0)
> - break;
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> - ret = put_user(child->thread.debug.dac1, datalp);
> -#else
> - dabr_fake = ((child->thread.hw_brk.address & 
> (~HW_BRK_TYPE_DABR)) |
> -  (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> - ret = put_user(dabr_fake, datalp);
> -#endif
> + case PTRACE_GET_DEBUGREG:
> + ret = ptrace_get_debugreg(child, addr, datalp);
>   break;
> - }
>  
>   case PTRACE_SET_DEBUGREG:
>   ret = ptrace_set_debugreg(child, addr, data);
> 

Otherwise,

Reviewed-by: Ravi Bangoria 



[RFC PATCH v2 10/12] powerpc/ptrace: create ptrace_get_debugreg()

2019-06-28 Thread Christophe Leroy
Create ptrace_get_debugreg() to handle PTRACE_GET_DEBUGREG and
reduce ifdef mess

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/kernel/ptrace/ptrace-adv.c   |  9 +
 arch/powerpc/kernel/ptrace/ptrace-decl.h  |  2 ++
 arch/powerpc/kernel/ptrace/ptrace-noadv.c | 13 +
 arch/powerpc/kernel/ptrace/ptrace.c   | 18 ++
 4 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c 
b/arch/powerpc/kernel/ptrace/ptrace-adv.c
index 86e71fa6c5c8..dcc765940344 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
@@ -83,6 +83,15 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
 }
 
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+   unsigned long __user *datalp)
+{
+   /* We only support one DABR and no IABRS at the moment */
+   if (addr > 0)
+   return -EINVAL;
+   return put_user(child->thread.debug.dac1, datalp);
+}
+
 int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned 
long data)
 {
/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h 
b/arch/powerpc/kernel/ptrace/ptrace-decl.h
index bdba09a87aea..4b4b6a1d508a 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
+++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
@@ -176,6 +176,8 @@ int tm_cgpr32_set(struct task_struct *target, const struct 
user_regset *regset,
 extern const struct user_regset_view user_ppc_native_view;
 
 /* ptrace-(no)adv */
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+   unsigned long __user *datalp);
 int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned 
long data);
 long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint 
*bp_info);
 long ppc_del_hwdebug(struct task_struct *child, long data);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c 
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 7db330c94538..985cca136f85 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -64,6 +64,19 @@ void user_disable_single_step(struct task_struct *task)
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
 }
 
+int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
+   unsigned long __user *datalp)
+{
+   unsigned long dabr_fake;
+
+   /* We only support one DABR and no IABRS at the moment */
+   if (addr > 0)
+   return -EINVAL;
+   dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
+(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+   return put_user(dabr_fake, datalp);
+}
+
 int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, unsigned 
long data)
 {
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c 
b/arch/powerpc/kernel/ptrace/ptrace.c
index 377e0e541d5f..e789afae6f56 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -211,23 +211,9 @@ long arch_ptrace(struct task_struct *child, long request,
break;
}
 
-   case PTRACE_GET_DEBUGREG: {
-#ifndef CONFIG_PPC_ADV_DEBUG_REGS
-   unsigned long dabr_fake;
-#endif
-   ret = -EINVAL;
-   /* We only support one DABR and no IABRS at the moment */
-   if (addr > 0)
-   break;
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
-   ret = put_user(child->thread.debug.dac1, datalp);
-#else
-   dabr_fake = ((child->thread.hw_brk.address & 
(~HW_BRK_TYPE_DABR)) |
-(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
-   ret = put_user(dabr_fake, datalp);
-#endif
+   case PTRACE_GET_DEBUGREG:
+   ret = ptrace_get_debugreg(child, addr, datalp);
break;
-   }
 
case PTRACE_SET_DEBUGREG:
ret = ptrace_set_debugreg(child, addr, data);
-- 
2.13.3