Re: [PATCH v2 4/5] powerpc: kprobes: factor out code to emulate instruction into a helper

[Naveen N. Rao]

Excerpts from Masami Hiramatsu's message of April 13, 2017 10:04:

On Wed, 12 Apr 2017 16:28:27 +0530
"Naveen N. Rao"  wrote:


This helper will be used in a subsequent patch to emulate instructions
on re-entering the kprobe handler. No functional change.


In this case, please merge this patch into the next patch which
actually uses the factored out function unless that changes
too much.


In hindsight, this patch actually just refactors the code so that the 
helper can be re-used subsequently. Using the helper constitutes a 
separate unrelated change, so I'm keeping this patch as is. I am 
updating the description to convey this better.


- Naveen



Thank you,



Acked-by: Ananth N Mavinakayanahalli 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 52 ++-
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 0732a0291ace..8b48f7d046bd 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -207,6 +207,35 @@ void __kprobes arch_prepare_kretprobe(struct 
kretprobe_instance *ri,
regs->link = (unsigned long)kretprobe_trampoline;
 }
 
+int __kprobes try_to_emulate(struct kprobe *p, struct pt_regs *regs)

+{
+   int ret;
+   unsigned int insn = *p->ainsn.insn;
+
+   /* regs->nip is also adjusted if emulate_step returns 1 */
+   ret = emulate_step(regs, insn);
+   if (ret > 0) {
+   /*
+* Once this instruction has been boosted
+* successfully, set the boostable flag
+*/
+   if (unlikely(p->ainsn.boostable == 0))
+   p->ainsn.boostable = 1;
+   } else if (ret < 0) {
+   /*
+* We don't allow kprobes on mtmsr(d)/rfi(d), etc.
+* So, we should never get here... but, its still
+* good to catch them, just in case...
+*/
+   printk("Can't step on instruction %x\n", insn);
+   BUG();
+   } else if (ret == 0)
+   /* This instruction can't be boosted */
+   p->ainsn.boostable = -1;
+
+   return ret;
+}
+
 int __kprobes kprobe_handler(struct pt_regs *regs)
 {
struct kprobe *p;
@@ -302,18 +331,9 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
 
 ss_probe:

if (p->ainsn.boostable >= 0) {
-   unsigned int insn = *p->ainsn.insn;
+   ret = try_to_emulate(p, regs);
 
-		/* regs->nip is also adjusted if emulate_step returns 1 */

-   ret = emulate_step(regs, insn);
if (ret > 0) {
-   /*
-* Once this instruction has been boosted
-* successfully, set the boostable flag
-*/
-   if (unlikely(p->ainsn.boostable == 0))
-   p->ainsn.boostable = 1;
-
if (p->post_handler)
p->post_handler(p, regs, 0);
 
@@ -321,17 +341,7 @@ int __kprobes kprobe_handler(struct pt_regs *regs)

reset_current_kprobe();
preempt_enable_no_resched();
return 1;
-   } else if (ret < 0) {
-   /*
-* We don't allow kprobes on mtmsr(d)/rfi(d), etc.
-* So, we should never get here... but, its still
-* good to catch them, just in case...
-*/
-   printk("Can't step on instruction %x\n", insn);
-   BUG();
-   } else if (ret == 0)
-   /* This instruction can't be boosted */
-   p->ainsn.boostable = -1;
+   }
}
prepare_singlestep(p, regs);
kcb->kprobe_status = KPROBE_HIT_SS;
--
2.12.1




--
Masami Hiramatsu 

Re: [PATCH v2 4/5] powerpc: kprobes: factor out code to emulate instruction into a helper

[Naveen N. Rao]

On 2017/04/13 01:34PM, Masami Hiramatsu wrote:
> On Wed, 12 Apr 2017 16:28:27 +0530
> "Naveen N. Rao"  wrote:
> 
> > This helper will be used in a subsequent patch to emulate instructions
> > on re-entering the kprobe handler. No functional change.
> 
> In this case, please merge this patch into the next patch which
> actually uses the factored out function unless that changes
> too much.

Ok, will do.

Thanks,
Naveen

Re: [PATCH v2 4/5] powerpc: kprobes: factor out code to emulate instruction into a helper

[Masami Hiramatsu]

On Wed, 12 Apr 2017 16:28:27 +0530
"Naveen N. Rao"  wrote:

> This helper will be used in a subsequent patch to emulate instructions
> on re-entering the kprobe handler. No functional change.

In this case, please merge this patch into the next patch which
actually uses the factored out function unless that changes
too much.

Thank you,

> 
> Acked-by: Ananth N Mavinakayanahalli 
> Signed-off-by: Naveen N. Rao 
> ---
>  arch/powerpc/kernel/kprobes.c | 52 
> ++-
>  1 file changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 0732a0291ace..8b48f7d046bd 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -207,6 +207,35 @@ void __kprobes arch_prepare_kretprobe(struct 
> kretprobe_instance *ri,
>   regs->link = (unsigned long)kretprobe_trampoline;
>  }
>  
> +int __kprobes try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> +{
> + int ret;
> + unsigned int insn = *p->ainsn.insn;
> +
> + /* regs->nip is also adjusted if emulate_step returns 1 */
> + ret = emulate_step(regs, insn);
> + if (ret > 0) {
> + /*
> +  * Once this instruction has been boosted
> +  * successfully, set the boostable flag
> +  */
> + if (unlikely(p->ainsn.boostable == 0))
> + p->ainsn.boostable = 1;
> + } else if (ret < 0) {
> + /*
> +  * We don't allow kprobes on mtmsr(d)/rfi(d), etc.
> +  * So, we should never get here... but, its still
> +  * good to catch them, just in case...
> +  */
> + printk("Can't step on instruction %x\n", insn);
> + BUG();
> + } else if (ret == 0)
> + /* This instruction can't be boosted */
> + p->ainsn.boostable = -1;
> +
> + return ret;
> +}
> +
>  int __kprobes kprobe_handler(struct pt_regs *regs)
>  {
>   struct kprobe *p;
> @@ -302,18 +331,9 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
>  
>  ss_probe:
>   if (p->ainsn.boostable >= 0) {
> - unsigned int insn = *p->ainsn.insn;
> + ret = try_to_emulate(p, regs);
>  
> - /* regs->nip is also adjusted if emulate_step returns 1 */
> - ret = emulate_step(regs, insn);
>   if (ret > 0) {
> - /*
> -  * Once this instruction has been boosted
> -  * successfully, set the boostable flag
> -  */
> - if (unlikely(p->ainsn.boostable == 0))
> - p->ainsn.boostable = 1;
> -
>   if (p->post_handler)
>   p->post_handler(p, regs, 0);
>  
> @@ -321,17 +341,7 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
>   reset_current_kprobe();
>   preempt_enable_no_resched();
>   return 1;
> - } else if (ret < 0) {
> - /*
> -  * We don't allow kprobes on mtmsr(d)/rfi(d), etc.
> -  * So, we should never get here... but, its still
> -  * good to catch them, just in case...
> -  */
> - printk("Can't step on instruction %x\n", insn);
> - BUG();
> - } else if (ret == 0)
> - /* This instruction can't be boosted */
> - p->ainsn.boostable = -1;
> + }
>   }
>   prepare_singlestep(p, regs);
>   kcb->kprobe_status = KPROBE_HIT_SS;
> -- 
> 2.12.1
> 


-- 
Masami Hiramatsu 

[PATCH v2 4/5] powerpc: kprobes: factor out code to emulate instruction into a helper

[Naveen N. Rao]

This helper will be used in a subsequent patch to emulate instructions
on re-entering the kprobe handler. No functional change.

Acked-by: Ananth N Mavinakayanahalli 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 52 ++-
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 0732a0291ace..8b48f7d046bd 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -207,6 +207,35 @@ void __kprobes arch_prepare_kretprobe(struct 
kretprobe_instance *ri,
regs->link = (unsigned long)kretprobe_trampoline;
 }
 
+int __kprobes try_to_emulate(struct kprobe *p, struct pt_regs *regs)
+{
+   int ret;
+   unsigned int insn = *p->ainsn.insn;
+
+   /* regs->nip is also adjusted if emulate_step returns 1 */
+   ret = emulate_step(regs, insn);
+   if (ret > 0) {
+   /*
+* Once this instruction has been boosted
+* successfully, set the boostable flag
+*/
+   if (unlikely(p->ainsn.boostable == 0))
+   p->ainsn.boostable = 1;
+   } else if (ret < 0) {
+   /*
+* We don't allow kprobes on mtmsr(d)/rfi(d), etc.
+* So, we should never get here... but, its still
+* good to catch them, just in case...
+*/
+   printk("Can't step on instruction %x\n", insn);
+   BUG();
+   } else if (ret == 0)
+   /* This instruction can't be boosted */
+   p->ainsn.boostable = -1;
+
+   return ret;
+}
+
 int __kprobes kprobe_handler(struct pt_regs *regs)
 {
struct kprobe *p;
@@ -302,18 +331,9 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
 
 ss_probe:
if (p->ainsn.boostable >= 0) {
-   unsigned int insn = *p->ainsn.insn;
+   ret = try_to_emulate(p, regs);
 
-   /* regs->nip is also adjusted if emulate_step returns 1 */
-   ret = emulate_step(regs, insn);
if (ret > 0) {
-   /*
-* Once this instruction has been boosted
-* successfully, set the boostable flag
-*/
-   if (unlikely(p->ainsn.boostable == 0))
-   p->ainsn.boostable = 1;
-
if (p->post_handler)
p->post_handler(p, regs, 0);
 
@@ -321,17 +341,7 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
reset_current_kprobe();
preempt_enable_no_resched();
return 1;
-   } else if (ret < 0) {
-   /*
-* We don't allow kprobes on mtmsr(d)/rfi(d), etc.
-* So, we should never get here... but, its still
-* good to catch them, just in case...
-*/
-   printk("Can't step on instruction %x\n", insn);
-   BUG();
-   } else if (ret == 0)
-   /* This instruction can't be boosted */
-   p->ainsn.boostable = -1;
+   }
}
prepare_singlestep(p, regs);
kcb->kprobe_status = KPROBE_HIT_SS;
-- 
2.12.1