Re: [PATCH V2 3/4] arch/powerpc: Implement Optprobes
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 b/arch/powerpc/kernel/Makefile index 1925341..54f0f47 100644 --- a/arch/powerpc/kernel/Makefile +++ b/a
Re: [PATCH V2 3/4] arch/powerpc: Implement Optprobes
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 a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile > index 1925341..54f0f47 100644 > --- a/arch/pow
[PATCH V2 3/4] arch/powerpc: Implement Optprobes
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 index 000..ecba221 --- /dev/null +++ b/arch/powerpc/kerne