Le 20/04/2021 à 08:51, Naveen N. Rao a écrit :
Christophe Leroy wrote:For that, create a 32 bits version of patch_imm64_load_insns() and create a patch_imm_load_insns() which calls patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns() on PPC64. Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead of raw ld/std, opt out things linked to paca and use stmw/lmw to save/restore registers. Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu> --- arch/powerpc/Kconfig | 2 +- arch/powerpc/kernel/optprobes.c | 24 +++++++++++++-- arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++--------- 3 files changed, 53 insertions(+), 19 deletions(-)Thanks for adding support for ppc32. It is good to see that it works without too many changes.diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index c1344c05226c..49b538e54efb 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -227,7 +227,7 @@ config PPC select HAVE_MOD_ARCH_SPECIFIC select HAVE_NMI if PERF_EVENTS || (PPC64 && PPC_BOOK3S) select HAVE_HARDLOCKUP_DETECTOR_ARCH if PPC64 && PPC_BOOK3S && SMP - select HAVE_OPTPROBES if PPC64 + select HAVE_OPTPROBES select HAVE_PERF_EVENTS select HAVE_PERF_EVENTS_NMI if PPC64select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCHdiff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c index 58fdb9f66e0f..cdf87086fa33 100644 --- a/arch/powerpc/kernel/optprobes.c +++ b/arch/powerpc/kernel/optprobes.c @@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op) } } +static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr) +{ + patch_instruction((struct ppc_inst *)addr, + ppc_inst(PPC_RAW_LIS(reg, IMM_H(val)))); + addr++; + + patch_instruction((struct ppc_inst *)addr, + ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val)))); +} + /* * Generate instructions to load provided immediate 64-bit value * to register 'reg' and patch these instructions at 'addr'. */ -static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr) +static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)Do you really need this?
Without it I get: from arch/powerpc/kernel/optprobes.c:8: arch/powerpc/kernel/optprobes.c: In function 'patch_imm64_load_insns':arch/powerpc/kernel/optprobes.c:163:14: error: right shift count >= width of type [-Werror=shift-count-overflow]
163 | ((val >> 48) & 0xffff))); | ^~ ./arch/powerpc/include/asm/inst.h:69:48: note: in definition of macro 'ppc_inst' 69 | #define ppc_inst(x) ((struct ppc_inst){ .val = x }) | ^arch/powerpc/kernel/optprobes.c:169:31: error: right shift count >= width of type [-Werror=shift-count-overflow]
169 | ___PPC_RS(reg) | ((val >> 32) & 0xffff))); | ^~ ./arch/powerpc/include/asm/inst.h:69:48: note: in definition of macro 'ppc_inst' 69 | #define ppc_inst(x) ((struct ppc_inst){ .val = x }) | ^
{ /* lis reg,(op)@highest */ patch_instruction((struct ppc_inst *)addr, @@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t * ___PPC_RS(reg) | (val & 0xffff))); } +static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr) +{ + if (IS_ENABLED(CONFIG_PPC64)) + patch_imm64_load_insns(val, reg, addr); + else + patch_imm32_load_insns(val, reg, addr); +} + int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) { struct ppc_inst branch_op_callback, branch_emulate_step, temp; @@ -230,7 +248,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) * Fixup the template with instructions to: * 1. load the address of the actual probepoint */ - patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX); + patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX); /* * 2. branch to optimized_callback() and emulate_step() @@ -264,7 +282,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) * 3. load instruction to be emulated into relevant register, and */ temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn); - patch_imm64_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX); + patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX); /* * 4. branch back from trampoline diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S index ff8ba4d3824d..49f31e554573 100644 --- a/arch/powerpc/kernel/optprobes_head.S +++ b/arch/powerpc/kernel/optprobes_head.S @@ -30,39 +30,47 @@ optinsn_slot: .global optprobe_template_entry optprobe_template_entry: /* Create an in-memory pt_regs */ - stdu r1,-INT_FRAME_SIZE(r1) + PPC_STLU r1,-INT_FRAME_SIZE(r1) SAVE_GPR(0,r1) /* Save the previous SP into stack */ addi r0,r1,INT_FRAME_SIZE - std r0,GPR1(r1) + PPC_STL r0,GPR1(r1) +#ifdef CONFIG_PPC64 SAVE_10GPRS(2,r1) SAVE_10GPRS(12,r1) SAVE_10GPRS(22,r1) +#else + stmw r2, GPR2(r1) +#endif /* Save SPRS */ mfmsr r5 - std r5,_MSR(r1) + PPC_STL r5,_MSR(r1) li r5,0x700 - std r5,_TRAP(r1) + PPC_STL r5,_TRAP(r1) li r5,0 - std r5,ORIG_GPR3(r1) - std r5,RESULT(r1) + PPC_STL r5,ORIG_GPR3(r1) + PPC_STL r5,RESULT(r1) mfctr r5 - std r5,_CTR(r1) + PPC_STL r5,_CTR(r1) mflr r5 - std r5,_LINK(r1) + PPC_STL r5,_LINK(r1) mfspr r5,SPRN_XER - std r5,_XER(r1) + PPC_STL r5,_XER(r1) mfcr r5 - std r5,_CCR(r1) + PPC_STL r5,_CCR(r1) +#ifdef CONFIG_PPC64 lbz r5,PACAIRQSOFTMASK(r13) std r5,SOFTE(r1) +#endif /* * We may get here from a module, so load the kernel TOC in r2. * The original TOC gets restored when pt_regs is restored * further below. */ +#ifdef CONFIG_PPC64 ld r2,PACATOC(r13) +#endifAre the ISA and ABI documents for ppc32 available publicly?
ABI: https://wiki.raptorcs.com/w/images/0/03/Power-Arch-32-bit-ABI-supp-1.0-Unified.pdf ISA: https://www.nxp.com/files-static/product/doc/MPCFPE32B.pdf
I would have thought that the TOC issues apply to ppc32 as well, but want to understand why this isn't a problem there.
r2 is the pointer to 'current' on PPC32. No TOC.
.global optprobe_template_op_address optprobe_template_op_address: @@ -72,9 +80,11 @@ optprobe_template_op_address: */ nop nop +#ifdef CONFIG_PPC64 nop nop nop +#endif /* 2. pt_regs pointer in r4 */ addi r4,r1,STACK_FRAME_OVERHEAD @@ -94,9 +104,11 @@ optprobe_template_insn: /* 2, Pass instruction to be emulated in r4 */ nop nop +#ifdef CONFIG_PPC64 nop nop nop +#endifIt would be nice to put these behind a macro so as to avoid these #ifdef blocks here, as well as with the register save/restore sequence.
Will see what I can do Christophe