bibo,mao wrote on Monday, October 30, 2006 9:56 PM
> On IA64 kprobe can not insert trap code on slot 1 because
> opcode of slot 1 crosses over two consecutive 8-bytes. On
> montecito machine 16 bytes atomic operation is avaiable,
> This patch implements 16 bytes atomic copy on montecito
> machine, so that kprobe can probe any slot on montecito
> machine.
>
> +/* this function uses st16/ld16 to atomically copy one bundle
> + * to code area, it requires src address and dest address is
> + * not in UC/UCE/WC area. Currently kernel physical memory
> + * identified map is cachable and WB, so there is no such check.
> + * input0: represents whether this cpu supports atomic
> + * st16/ld16 instruction
> + * input1: destionation address of bundle copy
> + * input2: source address of bundle copy
> + * return: -1 failed, 0 succeed
> + */
> +GLOBAL_ENTRY(kprobe_update_inst_bundle)
Hmm, the description doesn't match with implementation. I'm really
confused to the purpose of this asm function. It is using a pair of
ld8/st8 or using ld16/st16 depends on cpu feature. It returns error
only on address mis-alignment. There is no atomicity in there as
claimed in the description.
> @@ -460,10 +465,12 @@ void __kprobes arch_arm_kprobe(struct kp
> {
> unsigned long addr = (unsigned long)p->addr;
> unsigned long arm_addr = addr & ~0xFULL;
> + int atomic;
>
> + atomic = local_cpu_data->features & ITANIUM_CPUID4_AO;
> flush_icache_range((unsigned long)p->ainsn.insn,
> (unsigned long)p->ainsn.insn + sizeof(kprobe_opcode_t));
> - memcpy((char *)arm_addr, &p->opcode, sizeof(kprobe_opcode_t));
> + kprobe_update_inst_bundle(atomic, (void *)arm_addr, (void *)&p->opcode);
> flush_icache_range(arm_addr, arm_addr + sizeof(kprobe_opcode_t));
> }
Return value of kprobe_update_inst_bundle() is not used here. I suggest
re-design the function prototype. What does it mean to have an error? If
it is non-fatal, then why bother return a value?
> @@ -471,10 +478,11 @@ void __kprobes arch_disarm_kprobe(struct
> {
> unsigned long addr = (unsigned long)p->addr;
> unsigned long arm_addr = addr & ~0xFULL;
> + int atomic;
>
> + atomic = local_cpu_data->features & ITANIUM_CPUID4_AO;
> /* p->ainsn.insn contains the original unaltered kprobe_opcode_t */
> - memcpy((char *) arm_addr, (char *) p->ainsn.insn,
> - sizeof(kprobe_opcode_t));
> + kprobe_update_inst_bundle(atomic, (void *)arm_addr, (void *) p->ainsn.insn);
> flush_icache_range(arm_addr, arm_addr + sizeof(kprobe_opcode_t));
> }
Same here with kprobe_update_inst_bundle().
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|