This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] IA64 kprobe invalidate icache of jump buffer
- From: Keshavamurthy Anil S <anil dot s dot keshavamurthy at intel dot com>
- To: "bibo, mao" <bibo dot mao at intel dot com>
- Cc: linux-ia64 at vger dot kernel dot org, SystemTAP <systemtap at sources dot redhat dot com>, "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, Prasanna S Panchamukhi <prasanna at in dot ibm dot com>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Jim Keniston <jkenisto at us dot ibm dot com>, Masami Hiramatsu <hiramatu at sdl dot hitachi dot co dot jp>, davem at davemloft dot net
- Date: Fri, 30 Jun 2006 11:54:44 -0700
- Subject: Re: [PATCH] IA64 kprobe invalidate icache of jump buffer
- References: <44A482C9.3090607@intel.com>
- Reply-to: Keshavamurthy Anil S <anil dot s dot keshavamurthy at intel dot com>
On Fri, Jun 30, 2006 at 01:47:53AM +0000, bibo, mao wrote:
> Hi,
> Kprobe inserts breakpoint instruction in probepoint and then jumps
> to instruction slot when breakpoint is hit, the instruction slot icache
> must be consistent with dcache. Here is the patch which invalidates
> instruction slot icache area in IA64 platform.
> Without this patch, in some machines there will be fault when executing
> instruction slot where icache content is inconsistent with dcache.
>
> Signed-off-by: bibo,mao <bibo.mao@intel.com>
Bibo,
This patch looks lot better than your earlier one.
Please see minor comments below and once you fix them please
post the same onto lkml for inclusion.
>
> -------------------------------------------------------------------------------
>
> diff -Nruap 2.6.17.org/arch/ia64/kernel/kprobes.c 2.6.17/arch/ia64/kernel/kprobes.c
> --- 2.6.17.org/arch/ia64/kernel/kprobes.c 2006-06-29 03:50:15.000000000 +0800
> +++ 2.6.17/arch/ia64/kernel/kprobes.c 2006-06-30 02:24:42.000000000 +0800
> @@ -456,6 +456,8 @@ void __kprobes arch_arm_kprobe(struct kp
>
> memcpy((char *)arm_addr, &p->ainsn.insn.bundle, sizeof(bundle_t));
> flush_icache_range(arm_addr, arm_addr + sizeof(bundle_t));
> + arm_addr = (unsigned long)&p->opcode.bundle & ~0xFULL;
> + flush_icache_range(arm_addr, arm_addr + sizeof(bundle_t));
Please use flush_insn_slot() instead of the above two line change
and should move above memcpy() as you need to flush the jump buffer before
arming the probe.
> }
>
> void __kprobes arch_disarm_kprobe(struct kprobe *p)
> @@ -468,6 +470,14 @@ void __kprobes arch_disarm_kprobe(struct
> flush_icache_range(arm_addr, arm_addr + sizeof(bundle_t));
> }
>
> +void __kprobes flush_insn_slot(struct kprobe *p)
> +{
> + unsigned long arm_addr;
> +
> + arm_addr = ((unsigned long)&p->opcode.bundle) & ~0xFULL;
> + flush_icache_range(arm_addr, arm_addr + sizeof(bundle_t));
> +}
> +
> /*
> * We are resuming execution after a single step fault, so the pt_regs
> * structure reflects the register state after we executed the instruction
> diff -Nruap 2.6.17.org/include/asm-i386/kprobes.h 2.6.17/include/asm-i386/kprobes.h
> --- 2.6.17.org/include/asm-i386/kprobes.h 2006-06-29 03:50:18.000000000 +0800
> +++ 2.6.17/include/asm-i386/kprobes.h 2006-06-30 02:32:17.000000000 +0800
> @@ -44,6 +44,7 @@ typedef u8 kprobe_opcode_t;
>
> #define JPROBE_ENTRY(pentry) (kprobe_opcode_t *)pentry
> #define ARCH_SUPPORTS_KRETPROBES
> +#define flush_insn_slot(p) do { } while (0)
>
> void arch_remove_kprobe(struct kprobe *p);
> void kretprobe_trampoline(void);
> diff -Nruap 2.6.17.org/include/asm-ia64/kprobes.h 2.6.17/include/asm-ia64/kprobes.h
> --- 2.6.17.org/include/asm-ia64/kprobes.h 2006-03-27 14:41:22.000000000 +0800
> +++ 2.6.17/include/asm-ia64/kprobes.h 2006-06-30 02:33:04.000000000 +0800
> @@ -124,5 +124,6 @@ static inline void jprobe_return(void)
> }
> extern void invalidate_stacked_regs(void);
> extern void flush_register_stack(void);
> +extern void flush_insn_slot(struct kprobe *p);
>
> #endif /* _ASM_KPROBES_H */
> diff -Nruap 2.6.17.org/include/asm-powerpc/kprobes.h 2.6.17/include/asm-powerpc/kprobes.h
> --- 2.6.17.org/include/asm-powerpc/kprobes.h 2006-03-27 14:41:22.000000000 +0800
> +++ 2.6.17/include/asm-powerpc/kprobes.h 2006-06-30 02:32:34.000000000 +0800
> @@ -50,6 +50,7 @@ typedef unsigned int kprobe_opcode_t;
> IS_TWI(instr) || IS_TDI(instr))
>
> #define ARCH_SUPPORTS_KRETPROBES
> +#define flush_insn_slot(p) do { } while (0)
> void kretprobe_trampoline(void);
> extern void arch_remove_kprobe(struct kprobe *p);
>
> diff -Nruap 2.6.17.org/include/asm-sparc64/kprobes.h 2.6.17/include/asm-sparc64/kprobes.h
> --- 2.6.17.org/include/asm-sparc64/kprobes.h 2006-03-27 14:41:23.000000000 +0800
> +++ 2.6.17/include/asm-sparc64/kprobes.h 2006-06-30 02:32:50.000000000 +0800
> @@ -13,6 +13,7 @@ typedef u32 kprobe_opcode_t;
>
> #define JPROBE_ENTRY(pentry) (kprobe_opcode_t *)pentry
> #define arch_remove_kprobe(p) do {} while (0)
> +#define flush_insn_slot(p) do { } while (0)
>
> /* Architecture specific copy of original instruction*/
> struct arch_specific_insn {
> diff -Nruap 2.6.17.org/include/asm-x86_64/kprobes.h 2.6.17/include/asm-x86_64/kprobes.h
> --- 2.6.17.org/include/asm-x86_64/kprobes.h 2006-03-27 14:41:23.000000000 +0800
> +++ 2.6.17/include/asm-x86_64/kprobes.h 2006-06-30 02:32:05.000000000 +0800
> @@ -43,6 +43,7 @@ typedef u8 kprobe_opcode_t;
>
> #define JPROBE_ENTRY(pentry) (kprobe_opcode_t *)pentry
> #define ARCH_SUPPORTS_KRETPROBES
> +#define flush_insn_slot(p) do { } while (0)
>
> void kretprobe_trampoline(void);
> extern void arch_remove_kprobe(struct kprobe *p);
> diff -Nruap 2.6.17.org/kernel/kprobes.c 2.6.17/kernel/kprobes.c
> --- 2.6.17.org/kernel/kprobes.c 2006-06-29 03:50:19.000000000 +0800
> +++ 2.6.17/kernel/kprobes.c 2006-06-30 02:23:51.000000000 +0800
> @@ -420,6 +420,7 @@ static int __kprobes register_aggr_kprob
> add_aggr_kprobe(ap, old_p);
> copy_kprobe(ap, p);
> ret = add_new_kprobe(ap, p);
> + flush_insn_slot(ap);
Same here..you need to flush_insn_slot(ap) before add_new_kprobe(ap, p).
> }
> return ret;
> }