lkml.org 
[lkml]   [2014]   [Aug]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] kprobes: arm: enable OPTPROBES for ARM 32
(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?

>
> 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,

--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com




\
 
 \ /
  Last update: 2014-08-12 04:01    [W:0.063 / U:0.672 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site