Re: [PATCH V2 3/4] arch/powerpc: Implement Optprobes

2016-12-16 Thread Anju T Sudhakar

Hi Masami,



On Friday 16 December 2016 07:32 PM, Masami Hiramatsu wrote:

On Wed, 14 Dec 2016 21:48:27 +0530
Anju T Sudhakar  wrote:


Detour buffer contains instructions to create an in memory pt_regs.
After the execution of the pre-handler, a call is made for instruction 
emulation.
The NIP is determined in advanced through dummy instruction emulation and a 
branch
instruction is created to the NIP at the end of the trampoline.

Instruction slot for detour buffer is allocated from the reserved area.
For the time being, 64KB is reserved in memory for this purpose.

Instructions which can be emulated using analyse_instr() are suppliants
for optimization. Before optimization ensure that the address range
between the detour buffer allocated and the instruction being probed
is within ± 32MB.

Thank you for updating!
I think this has no critical issue, but I have some comments on it.
(just cleanup and hardenings)
Please see below.


Signed-off-by: Anju T Sudhakar 
Signed-off-by: Naveen N. Rao 
---
  .../features/debug/optprobes/arch-support.txt  |   2 +-
  arch/powerpc/Kconfig   |   1 +
  arch/powerpc/include/asm/kprobes.h |  23 +-
  arch/powerpc/include/asm/sstep.h   |   1 +
  arch/powerpc/kernel/Makefile   |   1 +
  arch/powerpc/kernel/optprobes.c| 333 +
  arch/powerpc/kernel/optprobes_head.S   | 135 +
  arch/powerpc/lib/sstep.c   |  22 ++
  8 files changed, 516 insertions(+), 2 deletions(-)
  create mode 100644 arch/powerpc/kernel/optprobes.c
  create mode 100644 arch/powerpc/kernel/optprobes_head.S

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..45bc99d 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
  |   nios2: | TODO |
  |openrisc: | TODO |
  |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  ok  |
  |s390: | TODO |
  |   score: | TODO |
  |  sh: | TODO |
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c7f120a..d563f0a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -98,6 +98,7 @@ config PPC
select HAVE_IOREMAP_PROT
select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && 
POWER7_CPU)
select HAVE_KPROBES
+   select HAVE_OPTPROBES if PPC64
select HAVE_ARCH_KGDB
select HAVE_KRETPROBES
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/powerpc/include/asm/kprobes.h 
b/arch/powerpc/include/asm/kprobes.h
index 2c9759bd..739ddc5 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -38,7 +38,22 @@ struct pt_regs;
  struct kprobe;
  
  typedef ppc_opcode_t kprobe_opcode_t;

-#define MAX_INSN_SIZE 1
+
+extern kprobe_opcode_t optinsn_slot;
+
+/* Optinsn template address */
+extern kprobe_opcode_t optprobe_template_entry[];
+extern kprobe_opcode_t optprobe_template_op_address[];
+extern kprobe_opcode_t optprobe_template_call_handler[];
+extern kprobe_opcode_t optprobe_template_insn[];
+extern kprobe_opcode_t optprobe_template_call_emulate[];
+extern kprobe_opcode_t optprobe_template_ret[];
+extern kprobe_opcode_t optprobe_template_end[];
+
+#define MAX_INSN_SIZE  1
+#define MAX_OPTIMIZED_LENGTH   4
+#define MAX_OPTINSN_SIZE   (optprobe_template_end - 
optprobe_template_entry)
+#define RELATIVEJUMP_SIZE  4

These size/length macros seems a bit odd. I guess MAX_INSN_SIZE
is based on sizeof(MAX_INSN_SIZE), but others are in byte.
Could you fix that? For example, define it with
sizeof(kprobe_opcode_t), and add comments on it, etc.


Sure. I will look into this and define it in  a consistent way.

  
  #ifdef PPC64_ELF_ABI_v2

  /* PPC64 ABIv2 needs local entry point */
@@ -124,6 +139,12 @@ struct kprobe_ctlblk {
struct prev_kprobe prev_kprobe;
  };
  
+struct arch_optimized_insn {

+   kprobe_opcode_t copied_insn[1];
+   /* detour buffer */
+   kprobe_opcode_t *insn;
+};
+
  extern int kprobe_exceptions_notify(struct notifier_block *self,
unsigned long val, void *data);
  extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index d3a42cc..f7ad425 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -87,3 +87,4 @@ struct instruction_op {
  
  extern int analyse_instr(struct instruction_op *op, struct pt_regs *regs,

 unsigned int instr);
+extern bool is_conditional_branch(unsigned int instr);
diff --git a/arch/powerpc/kernel/Makefile 

Re: [PATCH V2 3/4] arch/powerpc: Implement Optprobes

2016-12-16 Thread Masami Hiramatsu
On Wed, 14 Dec 2016 21:48:27 +0530
Anju T Sudhakar  wrote:

> Detour buffer contains instructions to create an in memory pt_regs.
> After the execution of the pre-handler, a call is made for instruction 
> emulation.
> The NIP is determined in advanced through dummy instruction emulation and a 
> branch
> instruction is created to the NIP at the end of the trampoline.
> 
> Instruction slot for detour buffer is allocated from the reserved area.
> For the time being, 64KB is reserved in memory for this purpose.
> 
> Instructions which can be emulated using analyse_instr() are suppliants
> for optimization. Before optimization ensure that the address range
> between the detour buffer allocated and the instruction being probed
> is within ± 32MB.

Thank you for updating!
I think this has no critical issue, but I have some comments on it.
(just cleanup and hardenings)
Please see below.

> 
> Signed-off-by: Anju T Sudhakar 
> Signed-off-by: Naveen N. Rao 
> ---
>  .../features/debug/optprobes/arch-support.txt  |   2 +-
>  arch/powerpc/Kconfig   |   1 +
>  arch/powerpc/include/asm/kprobes.h |  23 +-
>  arch/powerpc/include/asm/sstep.h   |   1 +
>  arch/powerpc/kernel/Makefile   |   1 +
>  arch/powerpc/kernel/optprobes.c| 333 
> +
>  arch/powerpc/kernel/optprobes_head.S   | 135 +
>  arch/powerpc/lib/sstep.c   |  22 ++
>  8 files changed, 516 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/kernel/optprobes.c
>  create mode 100644 arch/powerpc/kernel/optprobes_head.S
> 
> diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
> b/Documentation/features/debug/optprobes/arch-support.txt
> index b8999d8..45bc99d 100644
> --- a/Documentation/features/debug/optprobes/arch-support.txt
> +++ b/Documentation/features/debug/optprobes/arch-support.txt
> @@ -27,7 +27,7 @@
>  |   nios2: | TODO |
>  |openrisc: | TODO |
>  |  parisc: | TODO |
> -| powerpc: | TODO |
> +| powerpc: |  ok  |
>  |s390: | TODO |
>  |   score: | TODO |
>  |  sh: | TODO |
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c7f120a..d563f0a 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -98,6 +98,7 @@ config PPC
>   select HAVE_IOREMAP_PROT
>   select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && 
> POWER7_CPU)
>   select HAVE_KPROBES
> + select HAVE_OPTPROBES if PPC64
>   select HAVE_ARCH_KGDB
>   select HAVE_KRETPROBES
>   select HAVE_ARCH_TRACEHOOK
> diff --git a/arch/powerpc/include/asm/kprobes.h 
> b/arch/powerpc/include/asm/kprobes.h
> index 2c9759bd..739ddc5 100644
> --- a/arch/powerpc/include/asm/kprobes.h
> +++ b/arch/powerpc/include/asm/kprobes.h
> @@ -38,7 +38,22 @@ struct pt_regs;
>  struct kprobe;
>  
>  typedef ppc_opcode_t kprobe_opcode_t;
> -#define MAX_INSN_SIZE 1
> +
> +extern kprobe_opcode_t optinsn_slot;
> +
> +/* Optinsn template address */
> +extern kprobe_opcode_t optprobe_template_entry[];
> +extern kprobe_opcode_t optprobe_template_op_address[];
> +extern kprobe_opcode_t optprobe_template_call_handler[];
> +extern kprobe_opcode_t optprobe_template_insn[];
> +extern kprobe_opcode_t optprobe_template_call_emulate[];
> +extern kprobe_opcode_t optprobe_template_ret[];
> +extern kprobe_opcode_t optprobe_template_end[];
> +
> +#define MAX_INSN_SIZE1
> +#define MAX_OPTIMIZED_LENGTH 4
> +#define MAX_OPTINSN_SIZE (optprobe_template_end - 
> optprobe_template_entry)
> +#define RELATIVEJUMP_SIZE4

These size/length macros seems a bit odd. I guess MAX_INSN_SIZE
is based on sizeof(MAX_INSN_SIZE), but others are in byte.
Could you fix that? For example, define it with 
sizeof(kprobe_opcode_t), and add comments on it, etc.

>  
>  #ifdef PPC64_ELF_ABI_v2
>  /* PPC64 ABIv2 needs local entry point */
> @@ -124,6 +139,12 @@ struct kprobe_ctlblk {
>   struct prev_kprobe prev_kprobe;
>  };
>  
> +struct arch_optimized_insn {
> + kprobe_opcode_t copied_insn[1];
> + /* detour buffer */
> + kprobe_opcode_t *insn;
> +};
> +
>  extern int kprobe_exceptions_notify(struct notifier_block *self,
>   unsigned long val, void *data);
>  extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> diff --git a/arch/powerpc/include/asm/sstep.h 
> b/arch/powerpc/include/asm/sstep.h
> index d3a42cc..f7ad425 100644
> --- a/arch/powerpc/include/asm/sstep.h
> +++ b/arch/powerpc/include/asm/sstep.h
> @@ -87,3 +87,4 @@ struct instruction_op {
>  
>  extern int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
>unsigned int instr);
> +extern bool is_conditional_branch(unsigned int instr);
> diff --git 

[PATCH V2 3/4] arch/powerpc: Implement Optprobes

2016-12-14 Thread Anju T Sudhakar
Detour buffer contains instructions to create an in memory pt_regs.
After the execution of the pre-handler, a call is made for instruction 
emulation.
The NIP is determined in advanced through dummy instruction emulation and a 
branch
instruction is created to the NIP at the end of the trampoline.

Instruction slot for detour buffer is allocated from the reserved area.
For the time being, 64KB is reserved in memory for this purpose.

Instructions which can be emulated using analyse_instr() are suppliants
for optimization. Before optimization ensure that the address range
between the detour buffer allocated and the instruction being probed
is within ?? 32MB.

Signed-off-by: Anju T Sudhakar 
Signed-off-by: Naveen N. Rao 
---
 .../features/debug/optprobes/arch-support.txt  |   2 +-
 arch/powerpc/Kconfig   |   1 +
 arch/powerpc/include/asm/kprobes.h |  23 +-
 arch/powerpc/include/asm/sstep.h   |   1 +
 arch/powerpc/kernel/Makefile   |   1 +
 arch/powerpc/kernel/optprobes.c| 333 +
 arch/powerpc/kernel/optprobes_head.S   | 135 +
 arch/powerpc/lib/sstep.c   |  22 ++
 8 files changed, 516 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/optprobes.c
 create mode 100644 arch/powerpc/kernel/optprobes_head.S

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..45bc99d 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  ok  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c7f120a..d563f0a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -98,6 +98,7 @@ config PPC
select HAVE_IOREMAP_PROT
select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && 
POWER7_CPU)
select HAVE_KPROBES
+   select HAVE_OPTPROBES if PPC64
select HAVE_ARCH_KGDB
select HAVE_KRETPROBES
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/powerpc/include/asm/kprobes.h 
b/arch/powerpc/include/asm/kprobes.h
index 2c9759bd..739ddc5 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -38,7 +38,22 @@ struct pt_regs;
 struct kprobe;
 
 typedef ppc_opcode_t kprobe_opcode_t;
-#define MAX_INSN_SIZE 1
+
+extern kprobe_opcode_t optinsn_slot;
+
+/* Optinsn template address */
+extern kprobe_opcode_t optprobe_template_entry[];
+extern kprobe_opcode_t optprobe_template_op_address[];
+extern kprobe_opcode_t optprobe_template_call_handler[];
+extern kprobe_opcode_t optprobe_template_insn[];
+extern kprobe_opcode_t optprobe_template_call_emulate[];
+extern kprobe_opcode_t optprobe_template_ret[];
+extern kprobe_opcode_t optprobe_template_end[];
+
+#define MAX_INSN_SIZE  1
+#define MAX_OPTIMIZED_LENGTH   4
+#define MAX_OPTINSN_SIZE   (optprobe_template_end - 
optprobe_template_entry)
+#define RELATIVEJUMP_SIZE  4
 
 #ifdef PPC64_ELF_ABI_v2
 /* PPC64 ABIv2 needs local entry point */
@@ -124,6 +139,12 @@ struct kprobe_ctlblk {
struct prev_kprobe prev_kprobe;
 };
 
+struct arch_optimized_insn {
+   kprobe_opcode_t copied_insn[1];
+   /* detour buffer */
+   kprobe_opcode_t *insn;
+};
+
 extern int kprobe_exceptions_notify(struct notifier_block *self,
unsigned long val, void *data);
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index d3a42cc..f7ad425 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -87,3 +87,4 @@ struct instruction_op {
 
 extern int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 unsigned int instr);
+extern bool is_conditional_branch(unsigned int instr);
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1925341..54f0f47 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_KGDB)+= kgdb.o
 obj-$(CONFIG_BOOTX_TEXT)   += btext.o
 obj-$(CONFIG_SMP)  += smp.o
 obj-$(CONFIG_KPROBES)  += kprobes.o
+obj-$(CONFIG_OPTPROBES)+= optprobes.o optprobes_head.o
 obj-$(CONFIG_UPROBES)  += uprobes.o
 obj-$(CONFIG_PPC_UDBG_16550)   += legacy_serial.o udbg_16550.o
 obj-$(CONFIG_STACKTRACE)   += stacktrace.o
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
new file mode 100644