On 2014/8/12 9:38, Masami Hiramatsu wrote:
> (2014/08/11 22:48), Will Deacon wrote:
>> Hello,
>>
>> On Sat, Aug 09, 2014 at 03:12:19AM +0100, Wang Nan wrote:
>>> This patch introduce kprobeopt for ARM 32.
>>>
>>> Limitations:
>>> - Currently only kernel compiled with ARM ISA is supported.
>>>
>>> - Offset between probe point and optinsn slot must not larger than
>>> 32MiB. Masami Hiramatsu suggests replacing 2 words, it will make
>>> things complex. Futher patch can make such optimization.
>>>
>>> Kprobe opt on ARM is relatively simpler than kprobe opt on x86 because
>>> ARM instruction is always 4 bytes aligned and 4 bytes long. This patch
>>> replace probed instruction by a 'b', branch to trampoline code and then
>>> calls optimized_callback(). optimized_callback() calls opt_pre_handler()
>>> to execute kprobe handler. It also emulate/simulate replaced instruction.
>>
>> Could you briefly describe the optimisation please?
>
> On arm32, optimization means "replacing a breakpoint with a branch".
> Of course simple branch instruction doesn't memorize the source(probe)
> address, optprobe makes a trampoline code for each probe point and
> each trampoline stores "struct kprobe" of that probe point.
>
> At first, the kprobe puts a breakpoint into the probe site, and builds
> a trampoline. After a while, it starts optimizing the probe site by
> replacing the breakpoint with a branch.
>
>> I'm not familiar with
>> kprobes internals, but if you're trying to patch an arbitrary instruction
>> with a branch then that's not guaranteed to be atomic by the ARM
>> architecture.
>
> Hmm, I'm not sure about arm32 too. Would you mean patch_text() can't
> replace an instruction atomically? Or only the breakpoint is special?
> (for cache?)
> optprobe always swaps branch and breakpoint, isn't that safe?
>
Same question.
OPTPROBES always replace a breakpoint instruction to a branch, not "an arbitrary
instruction". Do you mean the previous breakpoint patching is unsafe?
__patch_text() uses
*(u32 *)addr = insn;
to patch an instruction, do you mean that it is unsafe?
ARM's kprobe and kprobeopt always use it to replace instructions. In some
special
case (a thumb instruction cross 2 words), it wraps such store using
stop_machine,
but in ARM case, it assume such store to be atomic.
>>
>> We can, however, patch branches with other branches.
>>
>> Anyway, minor comments in-line:
>>
>>> +/* Caller must ensure addr & 3 == 0 */
>>> +static int can_optimize(unsigned long paddr)
>>> +{
>>> + return 1;
>>> +}
>>
>> Why not check the paddr alignment here, rather than have a comment?
>
> Actually, we don't need to care about that. The alignment is already
> checked before calling this function (at arch_prepare_kprobe() in
> arch/arm/kernel/kprobes.c).
>
>>
>>> +/* Free optimized instruction slot */
>>> +static void
>>> +__arch_remove_optimized_kprobe(struct optimized_kprobe *op, int dirty)
>>> +{
>>> + if (op->optinsn.insn) {
>>> + free_optinsn_slot(op->optinsn.insn, dirty);
>>> + op->optinsn.insn = NULL;
>>> + }
>>> +}
>>> +
>>> +extern void kprobe_handler(struct pt_regs *regs);
>>> +
>>> +static void
>>> +optimized_callback(struct optimized_kprobe *op, struct pt_regs *regs)
>>> +{
>>> + unsigned long flags;
>>> + struct kprobe *p = &op->kp;
>>> + struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>>> +
>>> + /* Save skipped registers */
>>> + regs->ARM_pc = (unsigned long)op->kp.addr;
>>> + regs->ARM_ORIG_r0 = ~0UL;
>>
>> Why are you writing ORIG_r0?
>
> In x86, optimization(breakpoint to jump) is transparently done, thus
> we have to mimic all registers as the breakpoint exception. And in x86
> int3(which is the breakpoint) exception sets -1 to orig_ax.
> So, if arm32's breakpoint doesn't attach the ARM_ORIG_r0, you don't
> need to touch it. We just consider the pt_regs looks same as that
> at the breakpoint handler.
>
>>
>>> + local_irq_save(flags);
>>> +
>>> + if (kprobe_running()) {
>>> + kprobes_inc_nmissed_count(&op->kp);
>>> + } else {
>>> + __this_cpu_write(current_kprobe, &op->kp);
>>> + kcb->kprobe_status = KPROBE_HIT_ACTIVE;
>>> + opt_pre_handler(&op->kp, regs);
>>> + __this_cpu_write(current_kprobe, NULL);
>>> + }
>>> +
>>> + /* In each case, we must singlestep the replaced instruction. */
>>> + op->kp.ainsn.insn_singlestep(p->opcode, &p->ainsn, regs);
>>> +
>>> + local_irq_restore(flags);
>>> +}
>>> +
>>> +int arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>>> +{
>>> + u8 *buf;
>>> + unsigned long rel_chk;
>>> + unsigned long val;
>>> +
>>> + if (!can_optimize((unsigned long)op->kp.addr))
>>> + return -EILSEQ;
>>> +
>>> + op->optinsn.insn = get_optinsn_slot();
>>> + if (!op->optinsn.insn)
>>> + return -ENOMEM;
>>> +
>>> + /*
>>> + * Verify if the address gap is in 32MiB range, because this uses
>>> + * a relative jump.
>>> + *
>>> + * kprobe opt use a 'b' instruction to branch to optinsn.insn.
>>> + * According to ARM manual, branch instruction is:
>>> + *
>>> + * 31 28 27 24 23 0
>>> + * +------+---+---+---+---+----------------+
>>> + * | cond | 1 | 0 | 1 | 0 | imm24 |
>>> + * +------+---+---+---+---+----------------+
>>> + *
>>> + * imm24 is a signed 24 bits integer. The real branch offset is computed
>>> + * by: imm32 = SignExtend(imm24:'00', 32);
>>> + *
>>> + * So the maximum forward branch should be:
>>> + * (0x007fffff << 2) = 0x01fffffc = 0x1fffffc
>>> + * The maximum backword branch should be:
>>> + * (0xff800000 << 2) = 0xfe000000 = -0x2000000
>>> + *
>>> + * We can simply check (rel & 0xfe000003):
>>> + * if rel is positive, (rel & 0xfe000000) shoule be 0
>>> + * if rel is negitive, (rel & 0xfe000000) should be 0xfe000000
>>> + * the last '3' is used for alignment checking.
>>> + */
>>> + rel_chk = (unsigned long)((long)op->optinsn.insn -
>>> + (long)op->kp.addr + 8) & 0xfe000003;
>>> +
>>> + if ((rel_chk != 0) && (rel_chk != 0xfe000000)) {
>>> + __arch_remove_optimized_kprobe(op, 0);
>>> + return -ERANGE;
>>> + }
>>> +
>>> + buf = (u8 *)op->optinsn.insn;
>>> +
>>> + /* Copy arch-dep-instance from template */
>>> + memcpy(buf, &optprobe_template_entry, TMPL_END_IDX);
>>> +
>>> + /* Set probe information */
>>> + val = (unsigned long)op;
>>> + memcpy(buf + TMPL_VAL_IDX, &val, sizeof(val));
>>> +
>>> + /* Set probe function call */
>>> + val = (unsigned long)optimized_callback;
>>> + memcpy(buf + TMPL_CALL_IDX, &val, sizeof(val));
>>
>> Ok, so this is updating the `offset' portion of a b instruction, right? What
>> if memcpy does that byte-by-byte?
>
> No, as you can see a indirect call "blx r2" in optprobe_template_entry(
> inline asm), this sets .data bytes at optprobe_template_call which is loaded
> to r2 register. :-)
> So all the 4bytes are used for storing the address.
>
> Thank you,
>
However, the replaced code is an 'nop', may be it's misleading.
By the way, while reading __patch_text(), I find a bug in my v3 patch:
+ /*
+ * Backup instructions which will be replaced
+ * by jump address
+ */
+ memcpy(op->optinsn.copied_insn, op->kp.addr,
+ RELATIVEJUMP_SIZE);
+
Here, it seems we meed to use __opcode_to_mem_arm to translate.
Other memcpy in arch_prepare_optimized_kprobe() is no problem, because copied
stuff are value, not instruction.
I'll send a v4 patch to fix this problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
