Message ID | 20221224114315.850130-7-chenguokai17@mails.ucas.ac.cn (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Palmer Dabbelt |
Headers | show |
Series | Add OPTPROBES feature on RISCV | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
Chen Guokai <chenguokai17@mails.ucas.ac.cn> writes: > From: Liao Chang <liaochang1@huawei.com> > diff --git a/arch/riscv/kernel/probes/opt.c b/arch/riscv/kernel/probes/opt.c > index a0d2ab39e3fa..258a283c906d 100644 > --- a/arch/riscv/kernel/probes/opt.c > +++ b/arch/riscv/kernel/probes/opt.c > @@ -271,15 +271,103 @@ static void find_free_registers(struct kprobe *kp, struct optimized_kprobe *op, > *ra = (kw == 1UL) ? 0 : __builtin_ctzl(kw & ~1UL); > } > > +static bool insn_jump_into_range(unsigned long addr, unsigned long start, > + unsigned long end) > +{ > + kprobe_opcode_t insn = *(kprobe_opcode_t *)addr; > + unsigned long target, offset = GET_INSN_LENGTH(insn); > + > +#ifdef CONFIG_RISCV_ISA_C > + if (offset == RVC_INSN_LEN) { > + if (riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn)) > + target = addr + rvc_branch_imme(insn); > + else if (riscv_insn_is_c_jal(insn) || riscv_insn_is_c_j(insn)) > + target = addr + rvc_jal_imme(insn); > + else > + target = 0; > + return (target >= start) && (target < end); > + } > +#endif > + > + if (riscv_insn_is_branch(insn)) > + target = addr + rvi_branch_imme(insn); > + else if (riscv_insn_is_jal(insn)) > + target = addr + rvi_jal_imme(insn); > + else > + target = 0; > + return (target >= start) && (target < end); > +} > + > +static int search_copied_insn(unsigned long paddr, struct optimized_kprobe *op) > +{ > + int i = 1; > + unsigned long offset = GET_INSN_LENGTH(*(kprobe_opcode_t *)paddr); > + > + while ((i++ < MAX_COPIED_INSN) && (offset < 2 * RVI_INSN_LEN)) { > + if (riscv_probe_decode_insn((probe_opcode_t *)paddr + offset, > + NULL) != INSN_GOOD) If the second argument is NULL, and the insn is auipc, we'll splat with NULL-ptr exception. Hmm, probe_opcode_t is u32, right? And GET_INSN_LENGTH() returns 4 or 2 ...then the pointer arithmetic will be a mess? Björn
在 2023/1/3 2:04, Björn Töpel 写道: > Chen Guokai <chenguokai17@mails.ucas.ac.cn> writes: > >> From: Liao Chang <liaochang1@huawei.com> > >> diff --git a/arch/riscv/kernel/probes/opt.c b/arch/riscv/kernel/probes/opt.c >> index a0d2ab39e3fa..258a283c906d 100644 >> --- a/arch/riscv/kernel/probes/opt.c >> +++ b/arch/riscv/kernel/probes/opt.c >> @@ -271,15 +271,103 @@ static void find_free_registers(struct kprobe *kp, struct optimized_kprobe *op, >> *ra = (kw == 1UL) ? 0 : __builtin_ctzl(kw & ~1UL); >> } >> >> +static bool insn_jump_into_range(unsigned long addr, unsigned long start, >> + unsigned long end) >> +{ >> + kprobe_opcode_t insn = *(kprobe_opcode_t *)addr; >> + unsigned long target, offset = GET_INSN_LENGTH(insn); >> + >> +#ifdef CONFIG_RISCV_ISA_C >> + if (offset == RVC_INSN_LEN) { >> + if (riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn)) >> + target = addr + rvc_branch_imme(insn); >> + else if (riscv_insn_is_c_jal(insn) || riscv_insn_is_c_j(insn)) >> + target = addr + rvc_jal_imme(insn); >> + else >> + target = 0; >> + return (target >= start) && (target < end); >> + } >> +#endif >> + >> + if (riscv_insn_is_branch(insn)) >> + target = addr + rvi_branch_imme(insn); >> + else if (riscv_insn_is_jal(insn)) >> + target = addr + rvi_jal_imme(insn); >> + else >> + target = 0; >> + return (target >= start) && (target < end); >> +} >> + >> +static int search_copied_insn(unsigned long paddr, struct optimized_kprobe *op) >> +{ >> + int i = 1; >> + unsigned long offset = GET_INSN_LENGTH(*(kprobe_opcode_t *)paddr); >> + >> + while ((i++ < MAX_COPIED_INSN) && (offset < 2 * RVI_INSN_LEN)) { >> + if (riscv_probe_decode_insn((probe_opcode_t *)paddr + offset, >> + NULL) != INSN_GOOD) > > If the second argument is NULL, and the insn is auipc, we'll splat with > NULL-ptr exception. Good catch, it is my fault to ignore the access to second argument in macro RISCV_INSN_SET_SIMULATE. > > Hmm, probe_opcode_t is u32, right? And GET_INSN_LENGTH() returns 4 or 2 > ...then the pointer arithmetic will be a mess? Hmm, This pointer arithemtic does make no sense here, i had debugged this function on QEMU step by step, and it work well. Anyway, i will go through this function again, thanks. > > > Björn
diff --git a/arch/riscv/kernel/probes/opt.c b/arch/riscv/kernel/probes/opt.c index a0d2ab39e3fa..258a283c906d 100644 --- a/arch/riscv/kernel/probes/opt.c +++ b/arch/riscv/kernel/probes/opt.c @@ -271,15 +271,103 @@ static void find_free_registers(struct kprobe *kp, struct optimized_kprobe *op, *ra = (kw == 1UL) ? 0 : __builtin_ctzl(kw & ~1UL); } +static bool insn_jump_into_range(unsigned long addr, unsigned long start, + unsigned long end) +{ + kprobe_opcode_t insn = *(kprobe_opcode_t *)addr; + unsigned long target, offset = GET_INSN_LENGTH(insn); + +#ifdef CONFIG_RISCV_ISA_C + if (offset == RVC_INSN_LEN) { + if (riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn)) + target = addr + rvc_branch_imme(insn); + else if (riscv_insn_is_c_jal(insn) || riscv_insn_is_c_j(insn)) + target = addr + rvc_jal_imme(insn); + else + target = 0; + return (target >= start) && (target < end); + } +#endif + + if (riscv_insn_is_branch(insn)) + target = addr + rvi_branch_imme(insn); + else if (riscv_insn_is_jal(insn)) + target = addr + rvi_jal_imme(insn); + else + target = 0; + return (target >= start) && (target < end); +} + +static int search_copied_insn(unsigned long paddr, struct optimized_kprobe *op) +{ + int i = 1; + unsigned long offset = GET_INSN_LENGTH(*(kprobe_opcode_t *)paddr); + + while ((i++ < MAX_COPIED_INSN) && (offset < 2 * RVI_INSN_LEN)) { + if (riscv_probe_decode_insn((probe_opcode_t *)paddr + offset, + NULL) != INSN_GOOD) + return -1; + offset += GET_INSN_LENGTH(*(kprobe_opcode_t *)(paddr + offset)); + } + + op->optinsn.length = offset; + return 0; +} + /* - * If two free registers can be found at the beginning of both - * the start and the end of replaced code, it can be optimized - * Also, in-function jumps need to be checked to make sure that - * there is no jump to the second instruction to be replaced + * The kprobe can be optimized when no in-function jump reaches to the + * instructions replaced by optimized jump instructions(AUIPC/JALR). */ static bool can_optimize(unsigned long paddr, struct optimized_kprobe *op) { - return false; + int ret; + unsigned long addr, size = 0, offset = 0; + struct kprobe *kp = get_kprobe((kprobe_opcode_t *)paddr); + + /* + * Skip optimization if kprobe has been disarmed or instrumented + * instruction support XOI. + */ + if (!kp || (riscv_probe_decode_insn(&kp->opcode, NULL) != INSN_GOOD)) + return false; + + /* + * Find a instruction window large enough to contain a pair + * of AUIPC/JALR, and ensure each instruction in this window + * supports XOI. + */ + ret = search_copied_insn(paddr, op); + if (ret) + return false; + + if (!kallsyms_lookup_size_offset(paddr, &size, &offset)) + return false; + + /* Check there is enough space for relative jump(AUIPC/JALR) */ + if (size - offset <= op->optinsn.length) + return false; + + /* + * Decode instructions until function end, check any instruction + * don't jump into the window used to emit optprobe(AUIPC/JALR). + */ + addr = paddr - offset; + while (addr < paddr) { + if (insn_jump_into_range(addr, paddr + RVC_INSN_LEN, + paddr + op->optinsn.length)) + return false; + addr += GET_INSN_LENGTH(*(kprobe_opcode_t *)addr); + } + + addr = paddr + op->optinsn.length; + while (addr < paddr - offset + size) { + if (insn_jump_into_range(addr, paddr + RVC_INSN_LEN, + paddr + op->optinsn.length)) + return false; + addr += GET_INSN_LENGTH(*(kprobe_opcode_t *)addr); + } + + return true; } int arch_prepared_optinsn(struct arch_optimized_insn *optinsn)