[PATCHv2 0/6] ARM: jump label support

[PATCHv2 0/6] ARM: jump label support

[Rabin Vincent]

v2:
 - Use helper macros from Dave Martin's "ARM: Add generic instruction opcode
   manipulation helpers"
   http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7278/1

 - Add compiler bug detection so the option is not visible on unsupported
   compilers

 - patch.c: use uintptr_t, h for half-word, call cache_ops_need_broadcast()
   only once

 - Split ftrace.c patch into two

 - Remove nop instruction; use mov r0, r0 instead

Rabin Vincent (6):
  ARM: ftrace: remove useless memory checks
  ARM: ftrace: use canonical Thumb-2 wide instruction format
  ARM: extract out insn generation code from ftrace
  ARM: extract out code patch function from kprobes
  jump label: detect %c support for ARM
  ARM: add jump label support

 arch/arm/Kconfig                  |    1 +
 arch/arm/include/asm/jump_label.h |   41 +++++++++++++++++
 arch/arm/kernel/Makefile          |    9 +++-
 arch/arm/kernel/ftrace.c          |   90 +++++++++---------------------------
 arch/arm/kernel/insn.c            |   61 +++++++++++++++++++++++++
 arch/arm/kernel/insn.h            |   29 ++++++++++++
 arch/arm/kernel/jump_label.c      |   35 ++++++++++++++
 arch/arm/kernel/kprobes.c         |   86 ++++++++++-------------------------
 arch/arm/kernel/patch.c           |   73 ++++++++++++++++++++++++++++++
 arch/arm/kernel/patch.h           |    7 +++
 scripts/gcc-goto.sh               |   18 +++++++-
 11 files changed, 317 insertions(+), 133 deletions(-)
 create mode 100644 arch/arm/include/asm/jump_label.h
 create mode 100644 arch/arm/kernel/insn.c
 create mode 100644 arch/arm/kernel/insn.h
 create mode 100644 arch/arm/kernel/jump_label.c
 create mode 100644 arch/arm/kernel/patch.c
 create mode 100644 arch/arm/kernel/patch.h

-- 
1.7.8.3

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Rabin Vincent]

Before replacing an instruction, the ftrace code determines what the old
instruction should be and verifies that that's what's really there in
memory before replacing it.  This is useful if for example a bug in
mcountrecord causes it to record wrong locations.

However, in cases where we replace call sites in entry-common.S, these
checks are not needed.  For these, we currently just memcpy() the memory
content and then "verify" it -- this is quite useless and can be
removed.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/ftrace.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index c0062ad..e9488ad 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -125,11 +125,13 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 {
 	unsigned long replaced;
 
-	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
-		return -EFAULT;
+	if (old) {
+		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
+			return -EFAULT;
 
-	if (replaced != old)
-		return -EINVAL;
+		if (replaced != old)
+			return -EINVAL;
+	}
 
 	if (probe_kernel_write((void *)pc, &new, MCOUNT_INSN_SIZE))
 		return -EPERM;
@@ -141,23 +143,21 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
-	unsigned long pc, old;
+	unsigned long pc;
 	unsigned long new;
 	int ret;
 
 	pc = (unsigned long)&ftrace_call;
-	memcpy(&old, &ftrace_call, MCOUNT_INSN_SIZE);
 	new = ftrace_call_replace(pc, (unsigned long)func);
 
-	ret = ftrace_modify_code(pc, old, new);
+	ret = ftrace_modify_code(pc, 0, new);
 
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
-		memcpy(&old, &ftrace_call_old, MCOUNT_INSN_SIZE);
 		new = ftrace_call_replace(pc, (unsigned long)func);
 
-		ret = ftrace_modify_code(pc, old, new);
+		ret = ftrace_modify_code(pc, 0, new);
 	}
 #endif
 
-- 
1.7.8.3

[PATCHv2 2/6] ARM: ftrace: use canonical Thumb-2 wide instruction format

[Rabin Vincent]

As commit 592201a9f15 (ARM: Thumb-2: Support Thumb-2 in undefined
instruction handler) says:

    32-bit Thumb instructions are specified in the form:
        ((first_half << 16 ) | second_half)
    which matches the layout used by the ARM ARM.

Convert the ftrace code to use the same format to avoid the usage of
different formats in kernel code.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/ftrace.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index e9488ad..72a381a 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -16,10 +16,11 @@
 #include <linux/uaccess.h>
 
 #include <asm/cacheflush.h>
+#include <asm/opcodes.h>
 #include <asm/ftrace.h>
 
 #ifdef CONFIG_THUMB2_KERNEL
-#define	NOP		0xeb04f85d	/* pop.w {lr} */
+#define	NOP		0xf85deb04	/* pop.w {lr} */
 #else
 #define	NOP		0xe8bd4000	/* pop {lr} */
 #endif
@@ -88,7 +89,7 @@ static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
 	if (link)
 		second |= 1 << 14;
 
-	return (second << 16) | first;
+	return __opcode_thumb32_compose(first, second);
 }
 #else
 static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
@@ -125,6 +126,14 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 {
 	unsigned long replaced;
 
+	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
+		old = __opcode_to_mem_thumb32(old);
+		new = __opcode_to_mem_thumb32(new);
+	} else {
+		old = __opcode_to_mem_arm(old);
+		new = __opcode_to_mem_arm(new);
+	}
+
 	if (old) {
 		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
 			return -EFAULT;
-- 
1.7.8.3

[PATCHv2 3/6] ARM: extract out insn generation code from ftrace

[Rabin Vincent]

Extract out the instruction generation code so that it can be used
for jump labels too.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/Makefile |    5 ++-
 arch/arm/kernel/ftrace.c |   61 +++-------------------------------------------
 arch/arm/kernel/insn.c   |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/insn.h   |   19 ++++++++++++++
 4 files changed, 87 insertions(+), 59 deletions(-)
 create mode 100644 arch/arm/kernel/insn.c
 create mode 100644 arch/arm/kernel/insn.h

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 43b740d..dd3d5f1 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -7,6 +7,7 @@ AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
 
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_ftrace.o = -pg
+CFLAGS_REMOVE_insn.o = -pg
 endif
 
 CFLAGS_REMOVE_return_address.o = -pg
@@ -34,8 +35,8 @@ obj-$(CONFIG_HAVE_SCHED_CLOCK)	+= sched_clock.o
 obj-$(CONFIG_SMP)		+= smp.o smp_tlb.o
 obj-$(CONFIG_HAVE_ARM_SCU)	+= smp_scu.o
 obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
-obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
-obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
+obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
+obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o
 ifdef CONFIG_THUMB2_KERNEL
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 72a381a..858c63f 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -19,6 +19,8 @@
 #include <asm/opcodes.h>
 #include <asm/ftrace.h>
 
+#include "insn.h"
+
 #ifdef CONFIG_THUMB2_KERNEL
 #define	NOP		0xf85deb04	/* pop.w {lr} */
 #else
@@ -61,64 +63,9 @@ static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
 }
 #endif
 
-#ifdef CONFIG_THUMB2_KERNEL
-static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
-				       bool link)
-{
-	unsigned long s, j1, j2, i1, i2, imm10, imm11;
-	unsigned long first, second;
-	long offset;
-
-	offset = (long)addr - (long)(pc + 4);
-	if (offset < -16777216 || offset > 16777214) {
-		WARN_ON_ONCE(1);
-		return 0;
-	}
-
-	s	= (offset >> 24) & 0x1;
-	i1	= (offset >> 23) & 0x1;
-	i2	= (offset >> 22) & 0x1;
-	imm10	= (offset >> 12) & 0x3ff;
-	imm11	= (offset >>  1) & 0x7ff;
-
-	j1 = (!i1) ^ s;
-	j2 = (!i2) ^ s;
-
-	first = 0xf000 | (s << 10) | imm10;
-	second = 0x9000 | (j1 << 13) | (j2 << 11) | imm11;
-	if (link)
-		second |= 1 << 14;
-
-	return __opcode_thumb32_compose(first, second);
-}
-#else
-static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
-				       bool link)
-{
-	unsigned long opcode = 0xea000000;
-	long offset;
-
-	if (link)
-		opcode |= 1 << 24;
-
-	offset = (long)addr - (long)(pc + 8);
-	if (unlikely(offset < -33554432 || offset > 33554428)) {
-		/* Can't generate branches that far (from ARM ARM). Ftrace
-		 * doesn't generate branches outside of kernel text.
-		 */
-		WARN_ON_ONCE(1);
-		return 0;
-	}
-
-	offset = (offset >> 2) & 0x00ffffff;
-
-	return opcode | offset;
-}
-#endif
-
 static unsigned long ftrace_call_replace(unsigned long pc, unsigned long addr)
 {
-	return ftrace_gen_branch(pc, addr, true);
+	return arm_gen_branch_link(pc, addr);
 }
 
 static int ftrace_modify_code(unsigned long pc, unsigned long old,
@@ -258,7 +205,7 @@ static int __ftrace_modify_caller(unsigned long *callsite,
 {
 	unsigned long caller_fn = (unsigned long) func;
 	unsigned long pc = (unsigned long) callsite;
-	unsigned long branch = ftrace_gen_branch(pc, caller_fn, false);
+	unsigned long branch = arm_gen_branch(pc, caller_fn);
 	unsigned long nop = 0xe1a00000;	/* mov r0, r0 */
 	unsigned long old = enable ? nop : branch;
 	unsigned long new = enable ? branch : nop;
diff --git a/arch/arm/kernel/insn.c b/arch/arm/kernel/insn.c
new file mode 100644
index 0000000..ab312e5
--- /dev/null
+++ b/arch/arm/kernel/insn.c
@@ -0,0 +1,61 @@
+#include <linux/kernel.h>
+#include <asm/opcodes.h>
+
+static unsigned long
+__arm_gen_branch_thumb2(unsigned long pc, unsigned long addr, bool link)
+{
+	unsigned long s, j1, j2, i1, i2, imm10, imm11;
+	unsigned long first, second;
+	long offset;
+
+	offset = (long)addr - (long)(pc + 4);
+	if (offset < -16777216 || offset > 16777214) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}
+
+	s	= (offset >> 24) & 0x1;
+	i1	= (offset >> 23) & 0x1;
+	i2	= (offset >> 22) & 0x1;
+	imm10	= (offset >> 12) & 0x3ff;
+	imm11	= (offset >>  1) & 0x7ff;
+
+	j1 = (!i1) ^ s;
+	j2 = (!i2) ^ s;
+
+	first = 0xf000 | (s << 10) | imm10;
+	second = 0x9000 | (j1 << 13) | (j2 << 11) | imm11;
+	if (link)
+		second |= 1 << 14;
+
+	return __opcode_thumb32_compose(first, second);
+}
+
+static unsigned long
+__arm_gen_branch_arm(unsigned long pc, unsigned long addr, bool link)
+{
+	unsigned long opcode = 0xea000000;
+	long offset;
+
+	if (link)
+		opcode |= 1 << 24;
+
+	offset = (long)addr - (long)(pc + 8);
+	if (unlikely(offset < -33554432 || offset > 33554428)) {
+		WARN_ON_ONCE(1);
+		return 0;
+	}
+
+	offset = (offset >> 2) & 0x00ffffff;
+
+	return opcode | offset;
+}
+
+unsigned long
+__arm_gen_branch(unsigned long pc, unsigned long addr, bool link)
+{
+	if (IS_ENABLED(CONFIG_THUMB2_KERNEL))
+		return __arm_gen_branch_thumb2(pc, addr, link);
+	else
+		return __arm_gen_branch_arm(pc, addr, link);
+}
diff --git a/arch/arm/kernel/insn.h b/arch/arm/kernel/insn.h
new file mode 100644
index 0000000..994d60f
--- /dev/null
+++ b/arch/arm/kernel/insn.h
@@ -0,0 +1,19 @@
+#ifndef __ASM_ARM_INSN_H
+#define __ASM_ARM_INSN_H
+
+unsigned long
+__arm_gen_branch(unsigned long pc, unsigned long addr, bool link);
+
+static inline unsigned long
+arm_gen_branch(unsigned long pc, unsigned long addr)
+{
+	return __arm_gen_branch(pc, addr, false);
+}
+
+static inline unsigned long
+arm_gen_branch_link(unsigned long pc, unsigned long addr)
+{
+	return __arm_gen_branch(pc, addr, true);
+}
+
+#endif
-- 
1.7.8.3

[PATCHv2 4/6] ARM: extract out code patch function from kprobes

[Rabin Vincent]

Extract out the code patching code from kprobes so that it can be used
from the jump label code.  Additionally, the separated code:

 - Uses the IS_ENABLED() macros instead of the #ifdefs for THUMB2
   support

 - Unifies the two separate functions in kprobes, providing one function
   that uses stop_machine() internally, and one that can be called from
   stop_machine() directly

 - Patches the text on all CPUs only on processors requiring software
   broadcasting of cache operations

Cc: Jon Medhurst <tixy@yxit.co.uk>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/Makefile  |    3 +-
 arch/arm/kernel/kprobes.c |   86 ++++++++++++--------------------------------
 arch/arm/kernel/patch.c   |   73 ++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/patch.h   |    7 ++++
 4 files changed, 106 insertions(+), 63 deletions(-)
 create mode 100644 arch/arm/kernel/patch.c
 create mode 100644 arch/arm/kernel/patch.h

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index dd3d5f1..8c63ee8 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -8,6 +8,7 @@ AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg
+CFLAGS_REMOVE_patch.o = -pg
 endif
 
 CFLAGS_REMOVE_return_address.o = -pg
@@ -38,7 +39,7 @@ obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
-obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o
+obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o patch.o
 ifdef CONFIG_THUMB2_KERNEL
 obj-$(CONFIG_KPROBES)		+= kprobes-thumb.o
 else
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 129c116..ab1869d 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -29,6 +29,7 @@
 #include <asm/cacheflush.h>
 
 #include "kprobes.h"
+#include "patch.h"
 
 #define MIN_STACK_SIZE(addr) 				\
 	min((unsigned long)MAX_STACK_SIZE,		\
@@ -103,57 +104,33 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	return 0;
 }
 
-#ifdef CONFIG_THUMB2_KERNEL
-
-/*
- * For a 32-bit Thumb breakpoint spanning two memory words we need to take
- * special precautions to insert the breakpoint atomically, especially on SMP
- * systems. This is achieved by calling this arming function using stop_machine.
- */
-static int __kprobes set_t32_breakpoint(void *addr)
-{
-	((u16 *)addr)[0] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION >> 16;
-	((u16 *)addr)[1] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION & 0xffff;
-	flush_insns(addr, 2*sizeof(u16));
-	return 0;
-}
-
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	uintptr_t addr = (uintptr_t)p->addr & ~1; /* Remove any Thumb flag */
-
-	if (!is_wide_instruction(p->opcode)) {
-		*(u16 *)addr = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
-		flush_insns(addr, sizeof(u16));
-	} else if (addr & 2) {
-		/* A 32-bit instruction spanning two words needs special care */
-		stop_machine(set_t32_breakpoint, (void *)addr, &cpu_online_map);
+	unsigned int brkp;
+	void *addr;
+
+	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
+		/* Remove any Thumb flag */
+		addr = (void *)((uintptr_t)p->addr & ~1);
+
+		if (is_wide_instruction(p->opcode))
+			brkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
+		else
+			brkp = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
 	} else {
-		/* Word aligned 32-bit instruction can be written atomically */
-		u32 bkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
-#ifndef __ARMEB__ /* Swap halfwords for little-endian */
-		bkp = (bkp >> 16) | (bkp << 16);
-#endif
-		*(u32 *)addr = bkp;
-		flush_insns(addr, sizeof(u32));
-	}
-}
+		kprobe_opcode_t insn = p->opcode;
 
-#else /* !CONFIG_THUMB2_KERNEL */
+		addr = p->addr;
+		brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
 
-void __kprobes arch_arm_kprobe(struct kprobe *p)
-{
-	kprobe_opcode_t insn = p->opcode;
-	kprobe_opcode_t brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
-	if (insn >= 0xe0000000)
-		brkp |= 0xe0000000;  /* Unconditional instruction */
-	else
-		brkp |= insn & 0xf0000000;  /* Copy condition from insn */
-	*p->addr = brkp;
-	flush_insns(p->addr, sizeof(p->addr[0]));
-}
+		if (insn >= 0xe0000000)
+			brkp |= 0xe0000000;  /* Unconditional instruction */
+		else
+			brkp |= insn & 0xf0000000;  /* Copy condition from insn */
+	}
 
-#endif /* !CONFIG_THUMB2_KERNEL */
+	patch_text(addr, brkp);
+}
 
 /*
  * The actual disarming is done here on each CPU and synchronized using
@@ -166,25 +143,10 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
 int __kprobes __arch_disarm_kprobe(void *p)
 {
 	struct kprobe *kp = p;
-#ifdef CONFIG_THUMB2_KERNEL
-	u16 *addr = (u16 *)((uintptr_t)kp->addr & ~1);
-	kprobe_opcode_t insn = kp->opcode;
-	unsigned int len;
+	void *addr = (void *)((uintptr_t)kp->addr & ~1);
 
-	if (is_wide_instruction(insn)) {
-		((u16 *)addr)[0] = insn>>16;
-		((u16 *)addr)[1] = insn;
-		len = 2*sizeof(u16);
-	} else {
-		((u16 *)addr)[0] = insn;
-		len = sizeof(u16);
-	}
-	flush_insns(addr, len);
+	__patch_text(addr, kp->opcode);
 
-#else /* !CONFIG_THUMB2_KERNEL */
-	*kp->addr = kp->opcode;
-	flush_insns(kp->addr, sizeof(kp->addr[0]));
-#endif
 	return 0;
 }
 
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
new file mode 100644
index 0000000..30eff23
--- /dev/null
+++ b/arch/arm/kernel/patch.c
@@ -0,0 +1,73 @@
+#include <linux/kernel.h>
+#include <linux/kprobes.h>
+#include <linux/stop_machine.h>
+
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/opcodes.h>
+
+#include "patch.h"
+
+struct patch {
+	void *addr;
+	unsigned int insn;
+};
+
+void __kprobes __patch_text(void *addr, unsigned int insn)
+{
+	bool thumb2 = IS_ENABLED(CONFIG_THUMB2_KERNEL);
+	int size;
+
+	if (thumb2 && __opcode_is_thumb16(insn)) {
+		*(u16 *)addr = __opcode_to_mem_thumb16(insn);
+		size = sizeof(u16);
+	} else if (thumb2 && ((uintptr_t)addr & 2)) {
+		u16 *addrh = addr;
+
+		addrh[0] = __opcode_thumb32_first(insn);
+		addrh[1] = __opcode_thumb32_second(insn);
+
+		size = sizeof(u32);
+	} else {
+		if (thumb2)
+			insn = __opcode_to_mem_thumb32(insn);
+		else
+			insn = __opcode_to_mem_arm(insn);
+
+		*(u32 *)addr = insn;
+		size = sizeof(u32);
+	}
+
+	flush_icache_range((uintptr_t)(addr),
+			   (uintptr_t)(addr) + size);
+}
+
+static int __kprobes patch_text_stop_machine(void *data)
+{
+	struct patch *patch = data;
+
+	__patch_text(patch->addr, patch->insn);
+
+	return 0;
+}
+
+void __kprobes patch_text(void *addr, unsigned int insn)
+{
+	struct patch patch = {
+		.addr = addr,
+		.insn = insn,
+	};
+
+	if (cache_ops_need_broadcast()) {
+		stop_machine(patch_text_stop_machine, &patch, cpu_online_mask);
+	} else {
+		bool straddles_word = IS_ENABLED(CONFIG_THUMB2_KERNEL)
+				      && __opcode_is_thumb32(insn)
+				      && ((uintptr_t)addr & 2);
+
+		if (straddles_word)
+			stop_machine(patch_text_stop_machine, &patch, NULL);
+		else
+			__patch_text(addr, insn);
+	}
+}
diff --git a/arch/arm/kernel/patch.h b/arch/arm/kernel/patch.h
new file mode 100644
index 0000000..b4731f2
--- /dev/null
+++ b/arch/arm/kernel/patch.h
@@ -0,0 +1,7 @@
+#ifndef _ARM_KERNEL_PATCH_H
+#define _ARM_KERNEL_PATCH_H
+
+void patch_text(void *addr, unsigned int insn);
+void __patch_text(void *addr, unsigned int insn);
+
+#endif
-- 
1.7.8.3

[PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

Some versions of ARM GCC which do support asm goto, have problems
handling the the %c specifier.  Since we need the %c to support jump
labels on ARM, detect that too in the asm goto detection script to avoid
build errors with these versions.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637

Cc: Jason Baron <jbaron@redhat.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 scripts/gcc-goto.sh |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
index 98cffcb..9b744de 100644
--- a/scripts/gcc-goto.sh
+++ b/scripts/gcc-goto.sh
@@ -2,4 +2,20 @@
 # Test for gcc 'asm goto' support
 # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
 
-echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+int main(void)
+{
+#ifdef __arm__
+	/*
+	 * Not related to asm goto, but used by jump label
+	 * and broken on some ARM GCC versions (see GCC Bug 48637).
+	 */
+	static struct { int dummy; int state; } tp;
+	asm ("@ %c0" :: "i" (&tp.state));
+#endif
+
+entry:
+	asm goto ("" :::: entry);
+	return 0;
+}
+END
-- 
1.7.8.3

[PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

Some versions of ARM GCC which do support asm goto, have problems
handling the the %c specifier.  Since we need the %c to support jump
labels on ARM, detect that too in the asm goto detection script to avoid
build errors with these versions.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637

Cc: Jason Baron <jbaron@redhat.com>
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 scripts/gcc-goto.sh |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
index 98cffcb..9b744de 100644
--- a/scripts/gcc-goto.sh
+++ b/scripts/gcc-goto.sh
@@ -2,4 +2,20 @@
 # Test for gcc 'asm goto' support
 # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
 
-echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+int main(void)
+{
+#ifdef __arm__
+	/*
+	 * Not related to asm goto, but used by jump label
+	 * and broken on some ARM GCC versions (see GCC Bug 48637).
+	 */
+	static struct { int dummy; int state; } tp;
+	asm ("@ %c0" :: "i" (&tp.state));
+#endif
+
+entry:
+	asm goto ("" :::: entry);
+	return 0;
+}
+END
-- 
1.7.8.3

[PATCHv2 6/6] ARM: add jump label support

[Rabin Vincent]

Add the arch-specific code to support jump labels for ARM and Thumb-2.

The option will only be available on compilers that are capable of
building it.  It has been tested with GCC 4.6 patched with the patch
from GCC bug 48637.

Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/Kconfig                  |    1 +
 arch/arm/include/asm/jump_label.h |   41 +++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/Makefile          |    1 +
 arch/arm/kernel/insn.h            |   10 +++++++++
 arch/arm/kernel/jump_label.c      |   35 +++++++++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/jump_label.h
 create mode 100644 arch/arm/kernel/jump_label.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 24626b0..a1c8911 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -9,6 +9,7 @@ config ARM
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_ATOMIC64 if (CPU_V6 || !CPU_32v6K || !AEABI)
 	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
+	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
 	select HAVE_ARCH_KGDB
 	select HAVE_KPROBES if !XIP_KERNEL
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
new file mode 100644
index 0000000..5c5ca2e
--- /dev/null
+++ b/arch/arm/include/asm/jump_label.h
@@ -0,0 +1,41 @@
+#ifndef _ASM_ARM_JUMP_LABEL_H
+#define _ASM_ARM_JUMP_LABEL_H
+
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <asm/system.h>
+
+#define JUMP_LABEL_NOP_SIZE 4
+
+#ifdef CONFIG_THUMB2_KERNEL
+#define JUMP_LABEL_NOP	"nop.w"
+#else
+#define JUMP_LABEL_NOP	"nop"
+#endif
+
+static __always_inline bool arch_static_branch(struct jump_label_key *key)
+{
+	asm goto("1:\n\t"
+		 JUMP_LABEL_NOP "\n\t"
+		 ".pushsection __jump_table,  \"aw\"\n\t"
+		 ".word 1b, %l[l_yes], %c0\n\t"
+		 ".popsection\n\t"
+		 : :  "i" (key) :  : l_yes);
+
+	return false;
+l_yes:
+	return true;
+}
+
+#endif /* __KERNEL__ */
+
+typedef u32 jump_label_t;
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#endif
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 8c63ee8..1b7d9a3 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_HAVE_ARM_SCU)	+= smp_scu.o
 obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
+obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o insn.o patch.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o patch.o
 ifdef CONFIG_THUMB2_KERNEL
diff --git a/arch/arm/kernel/insn.h b/arch/arm/kernel/insn.h
index 994d60f..e96065d 100644
--- a/arch/arm/kernel/insn.h
+++ b/arch/arm/kernel/insn.h
@@ -1,6 +1,16 @@
 #ifndef __ASM_ARM_INSN_H
 #define __ASM_ARM_INSN_H
 
+static inline unsigned long
+arm_gen_nop(void)
+{
+#ifdef CONFIG_THUMB2_KERNEL
+	return 0xf3af8000; /* nop.w */
+#else
+	return 0xe1a00000; /* mov r0, r0 */
+#endif
+}
+
 unsigned long
 __arm_gen_branch(unsigned long pc, unsigned long addr, bool link);
 
diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
new file mode 100644
index 0000000..7900914
--- /dev/null
+++ b/arch/arm/kernel/jump_label.c
@@ -0,0 +1,35 @@
+#include <linux/kernel.h>
+#include <linux/jump_label.h>
+
+#include "insn.h"
+#include "patch.h"
+
+static void __arch_jump_label_transform(struct jump_entry *entry,
+					enum jump_label_type type,
+					bool is_static)
+{
+	void *addr = (void *)entry->code;
+	unsigned int insn;
+
+	if (type == JUMP_LABEL_ENABLE)
+		insn = arm_gen_branch(entry->code, entry->target);
+	else
+		insn = arm_gen_nop();
+
+	if (is_static)
+		__patch_text(addr, insn);
+	else
+		patch_text(addr, insn);
+}
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
+	__arch_jump_label_transform(entry, type, false);
+}
+
+void arch_jump_label_transform_static(struct jump_entry *entry,
+				      enum jump_label_type type)
+{
+	__arch_jump_label_transform(entry, type, true);
+}
-- 
1.7.8.3

[PATCHv2 2/6] ARM: ftrace: use canonical Thumb-2 wide instruction format

[Dave Martin]

On Sat, Jan 28, 2012 at 07:05:21PM +0530, Rabin Vincent wrote:
> As commit 592201a9f15 (ARM: Thumb-2: Support Thumb-2 in undefined
> instruction handler) says:
> 
>     32-bit Thumb instructions are specified in the form:
>         ((first_half << 16 ) | second_half)
>     which matches the layout used by the ARM ARM.
> 
> Convert the ftrace code to use the same format to avoid the usage of
> different formats in kernel code.
> 
> Signed-off-by: Rabin Vincent <rabin@rab.in>

Acked-by: Dave Martin <dave.martin@linaro.org>

I haven't tried it out, but the use of the opcode helpers looks correct.

Cheers
---Dave

> ---
>  arch/arm/kernel/ftrace.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index e9488ad..72a381a 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -16,10 +16,11 @@
>  #include <linux/uaccess.h>
>  
>  #include <asm/cacheflush.h>
> +#include <asm/opcodes.h>
>  #include <asm/ftrace.h>
>  
>  #ifdef CONFIG_THUMB2_KERNEL
> -#define	NOP		0xeb04f85d	/* pop.w {lr} */
> +#define	NOP		0xf85deb04	/* pop.w {lr} */
>  #else
>  #define	NOP		0xe8bd4000	/* pop {lr} */
>  #endif
> @@ -88,7 +89,7 @@ static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
>  	if (link)
>  		second |= 1 << 14;
>  
> -	return (second << 16) | first;
> +	return __opcode_thumb32_compose(first, second);
>  }
>  #else
>  static unsigned long ftrace_gen_branch(unsigned long pc, unsigned long addr,
> @@ -125,6 +126,14 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
>  {
>  	unsigned long replaced;
>  
> +	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
> +		old = __opcode_to_mem_thumb32(old);
> +		new = __opcode_to_mem_thumb32(new);
> +	} else {
> +		old = __opcode_to_mem_arm(old);
> +		new = __opcode_to_mem_arm(new);
> +	}
> +
>  	if (old) {
>  		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
>  			return -EFAULT;
> -- 
> 1.7.8.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv2 4/6] ARM: extract out code patch function from kprobes

[Dave Martin]

On Sat, Jan 28, 2012 at 07:05:23PM +0530, Rabin Vincent wrote:
> Extract out the code patching code from kprobes so that it can be used
> from the jump label code.  Additionally, the separated code:
> 
>  - Uses the IS_ENABLED() macros instead of the #ifdefs for THUMB2
>    support
> 
>  - Unifies the two separate functions in kprobes, providing one function
>    that uses stop_machine() internally, and one that can be called from
>    stop_machine() directly
> 
>  - Patches the text on all CPUs only on processors requiring software
>    broadcasting of cache operations
> 
> Cc: Jon Medhurst <tixy@yxit.co.uk>
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  arch/arm/kernel/Makefile  |    3 +-
>  arch/arm/kernel/kprobes.c |   86 ++++++++++++--------------------------------
>  arch/arm/kernel/patch.c   |   73 ++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/patch.h   |    7 ++++
>  4 files changed, 106 insertions(+), 63 deletions(-)
>  create mode 100644 arch/arm/kernel/patch.c
>  create mode 100644 arch/arm/kernel/patch.h

[...]

> diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
> new file mode 100644
> index 0000000..30eff23
> --- /dev/null
> +++ b/arch/arm/kernel/patch.c
> @@ -0,0 +1,73 @@

[...]

> +void __kprobes __patch_text(void *addr, unsigned int insn)
> +{
> +	bool thumb2 = IS_ENABLED(CONFIG_THUMB2_KERNEL);
> +	int size;
> +
> +	if (thumb2 && __opcode_is_thumb16(insn)) {
> +		*(u16 *)addr = __opcode_to_mem_thumb16(insn);
> +		size = sizeof(u16);
> +	} else if (thumb2 && ((uintptr_t)addr & 2)) {
> +		u16 *addrh = addr;
> +
> +		addrh[0] = __opcode_thumb32_first(insn);
> +		addrh[1] = __opcode_thumb32_second(insn);

It looks like we never convert to memory byte order in this case.

If not, should this be as follows?

	addrh[0] = __opcode_to_mem_thumb16(__opcode_thumb32_first(insn));
	addrh[1] = __opcode_to_mem_thumb16(__opcode_thumb32_second(insn));

> +
> +		size = sizeof(u32);
> +	} else {
> +		if (thumb2)
> +			insn = __opcode_to_mem_thumb32(insn);
> +		else
> +			insn = __opcode_to_mem_arm(insn);
> +
> +		*(u32 *)addr = insn;
> +		size = sizeof(u32);
> +	}
> +
> +	flush_icache_range((uintptr_t)(addr),
> +			   (uintptr_t)(addr) + size);
> +}

[...]

Cheers
---Dave

[PATCHv2 0/6] ARM: jump label support

[Jon Medhurst (Tixy)]

On Sat, 2012-01-28 at 19:05 +0530, Rabin Vincent wrote:
> v2:
>  - Use helper macros from Dave Martin's "ARM: Add generic instruction opcode
>    manipulation helpers"
>    http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7278/1

This patch is broken, it contains two diff listings for
arch/arm/include/asm/opcodes.h

The second one looks more correct as the diff accounts for commit
0c9030deaf59d444 (ARM: 7206/1: Add generic ARM instruction set condition
code checks). However, when opcodes.h is included in assembler files it
produces error as it #includes C headers.

Moving the #ifndef __ASSEMBLY__ to above the #includes makes things
compile cleanly...

+#ifndef __ASSEMBLY__
 #include <linux/types.h>
 #include <linux/swab.h>

-#ifndef __ASSEMBLY__


-- 
Tixy

[PATCHv2 0/6] ARM: jump label support

[Dave Martin]

On Tue, Jan 31, 2012 at 08:23:14AM +0000, Jon Medhurst (Tixy) wrote:
> On Sat, 2012-01-28 at 19:05 +0530, Rabin Vincent wrote:
> > v2:
> >  - Use helper macros from Dave Martin's "ARM: Add generic instruction opcode
> >    manipulation helpers"
> >    http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7278/1
> 
> This patch is broken, it contains two diff listings for
> arch/arm/include/asm/opcodes.h
> 
> The second one looks more correct as the diff accounts for commit
> 0c9030deaf59d444 (ARM: 7206/1: Add generic ARM instruction set condition
> code checks). However, when opcodes.h is included in assembler files it
> produces error as it #includes C headers.

Erm, I'm really not sure what happened there...  it looks like I somehow
pasted together an old and a new submission in one file whem I submittedo
it.  The mind boggles.

> 
> Moving the #ifndef __ASSEMBLY__ to above the #includes makes things
> compile cleanly...
> 
> +#ifndef __ASSEMBLY__
>  #include <linux/types.h>
>  #include <linux/swab.h>
> 
> -#ifndef __ASSEMBLY__

This is straightforwardly fixed -- I'll fix these issues and post back to
the list.


Now reposted here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/082603.html

When people have had a chance to review, I'll update the patch in the
patch system.

Cheers
---Dave

[PATCHv2 4/6] ARM: extract out code patch function from kprobes

[Tixy]

On Sat, 2012-01-28 at 19:05 +0530, Rabin Vincent wrote:
> Extract out the code patching code from kprobes so that it can be used
> from the jump label code.  Additionally, the separated code:
> 
>  - Uses the IS_ENABLED() macros instead of the #ifdefs for THUMB2
>    support
> 
>  - Unifies the two separate functions in kprobes, providing one function
>    that uses stop_machine() internally, and one that can be called from
>    stop_machine() directly
> 
>  - Patches the text on all CPUs only on processors requiring software
>    broadcasting of cache operations
> 
> Cc: Jon Medhurst <tixy@yxit.co.uk>
> Signed-off-by: Rabin Vincent <rabin@rab.in>

Tested-by: Jon Medhurst <tixy@yxit.co.uk>

I haven't tested on a big endian machine, but from code review, Dave's
suggested change to __patch_text() looks like it is needed...
http://lists.arm.linux.org.uk/lurker/message/20120130.170006.8dea4fe6.en.html

With this change, these kprobes patches are
Acked-by: Jon Medhurst <tixy@yxit.co.uk>


> ---
>  arch/arm/kernel/Makefile  |    3 +-
>  arch/arm/kernel/kprobes.c |   86 ++++++++++++--------------------------------
>  arch/arm/kernel/patch.c   |   73 ++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/patch.h   |    7 ++++
>  4 files changed, 106 insertions(+), 63 deletions(-)
>  create mode 100644 arch/arm/kernel/patch.c
>  create mode 100644 arch/arm/kernel/patch.h
> 
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index dd3d5f1..8c63ee8 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -8,6 +8,7 @@ AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  ifdef CONFIG_FUNCTION_TRACER
>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg
> +CFLAGS_REMOVE_patch.o = -pg
>  endif
>  
>  CFLAGS_REMOVE_return_address.o = -pg
> @@ -38,7 +39,7 @@ obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
>  obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
>  obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
>  obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
> -obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o
> +obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o patch.o
>  ifdef CONFIG_THUMB2_KERNEL
>  obj-$(CONFIG_KPROBES)		+= kprobes-thumb.o
>  else
> diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> index 129c116..ab1869d 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -29,6 +29,7 @@
>  #include <asm/cacheflush.h>
>  
>  #include "kprobes.h"
> +#include "patch.h"
>  
>  #define MIN_STACK_SIZE(addr) 				\
>  	min((unsigned long)MAX_STACK_SIZE,		\
> @@ -103,57 +104,33 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_THUMB2_KERNEL
> -
> -/*
> - * For a 32-bit Thumb breakpoint spanning two memory words we need to take
> - * special precautions to insert the breakpoint atomically, especially on SMP
> - * systems. This is achieved by calling this arming function using stop_machine.
> - */
> -static int __kprobes set_t32_breakpoint(void *addr)
> -{
> -	((u16 *)addr)[0] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION >> 16;
> -	((u16 *)addr)[1] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION & 0xffff;
> -	flush_insns(addr, 2*sizeof(u16));
> -	return 0;
> -}
> -
>  void __kprobes arch_arm_kprobe(struct kprobe *p)
>  {
> -	uintptr_t addr = (uintptr_t)p->addr & ~1; /* Remove any Thumb flag */
> -
> -	if (!is_wide_instruction(p->opcode)) {
> -		*(u16 *)addr = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
> -		flush_insns(addr, sizeof(u16));
> -	} else if (addr & 2) {
> -		/* A 32-bit instruction spanning two words needs special care */
> -		stop_machine(set_t32_breakpoint, (void *)addr, &cpu_online_map);
> +	unsigned int brkp;
> +	void *addr;
> +
> +	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
> +		/* Remove any Thumb flag */
> +		addr = (void *)((uintptr_t)p->addr & ~1);
> +
> +		if (is_wide_instruction(p->opcode))
> +			brkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
> +		else
> +			brkp = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
>  	} else {
> -		/* Word aligned 32-bit instruction can be written atomically */
> -		u32 bkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
> -#ifndef __ARMEB__ /* Swap halfwords for little-endian */
> -		bkp = (bkp >> 16) | (bkp << 16);
> -#endif
> -		*(u32 *)addr = bkp;
> -		flush_insns(addr, sizeof(u32));
> -	}
> -}
> +		kprobe_opcode_t insn = p->opcode;
>  
> -#else /* !CONFIG_THUMB2_KERNEL */
> +		addr = p->addr;
> +		brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
>  
> -void __kprobes arch_arm_kprobe(struct kprobe *p)
> -{
> -	kprobe_opcode_t insn = p->opcode;
> -	kprobe_opcode_t brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
> -	if (insn >= 0xe0000000)
> -		brkp |= 0xe0000000;  /* Unconditional instruction */
> -	else
> -		brkp |= insn & 0xf0000000;  /* Copy condition from insn */
> -	*p->addr = brkp;
> -	flush_insns(p->addr, sizeof(p->addr[0]));
> -}
> +		if (insn >= 0xe0000000)
> +			brkp |= 0xe0000000;  /* Unconditional instruction */
> +		else
> +			brkp |= insn & 0xf0000000;  /* Copy condition from insn */
> +	}
>  
> -#endif /* !CONFIG_THUMB2_KERNEL */
> +	patch_text(addr, brkp);
> +}
>  
>  /*
>   * The actual disarming is done here on each CPU and synchronized using
> @@ -166,25 +143,10 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
>  int __kprobes __arch_disarm_kprobe(void *p)
>  {
>  	struct kprobe *kp = p;
> -#ifdef CONFIG_THUMB2_KERNEL
> -	u16 *addr = (u16 *)((uintptr_t)kp->addr & ~1);
> -	kprobe_opcode_t insn = kp->opcode;
> -	unsigned int len;
> +	void *addr = (void *)((uintptr_t)kp->addr & ~1);
>  
> -	if (is_wide_instruction(insn)) {
> -		((u16 *)addr)[0] = insn>>16;
> -		((u16 *)addr)[1] = insn;
> -		len = 2*sizeof(u16);
> -	} else {
> -		((u16 *)addr)[0] = insn;
> -		len = sizeof(u16);
> -	}
> -	flush_insns(addr, len);
> +	__patch_text(addr, kp->opcode);
>  
> -#else /* !CONFIG_THUMB2_KERNEL */
> -	*kp->addr = kp->opcode;
> -	flush_insns(kp->addr, sizeof(kp->addr[0]));
> -#endif
>  	return 0;
>  }
>  
> diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
> new file mode 100644
> index 0000000..30eff23
> --- /dev/null
> +++ b/arch/arm/kernel/patch.c
> @@ -0,0 +1,73 @@
> +#include <linux/kernel.h>
> +#include <linux/kprobes.h>
> +#include <linux/stop_machine.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/smp_plat.h>
> +#include <asm/opcodes.h>
> +
> +#include "patch.h"
> +
> +struct patch {
> +	void *addr;
> +	unsigned int insn;
> +};
> +
> +void __kprobes __patch_text(void *addr, unsigned int insn)
> +{
> +	bool thumb2 = IS_ENABLED(CONFIG_THUMB2_KERNEL);
> +	int size;
> +
> +	if (thumb2 && __opcode_is_thumb16(insn)) {
> +		*(u16 *)addr = __opcode_to_mem_thumb16(insn);
> +		size = sizeof(u16);
> +	} else if (thumb2 && ((uintptr_t)addr & 2)) {
> +		u16 *addrh = addr;
> +
> +		addrh[0] = __opcode_thumb32_first(insn);
> +		addrh[1] = __opcode_thumb32_second(insn);
> +
> +		size = sizeof(u32);
> +	} else {
> +		if (thumb2)
> +			insn = __opcode_to_mem_thumb32(insn);
> +		else
> +			insn = __opcode_to_mem_arm(insn);
> +
> +		*(u32 *)addr = insn;
> +		size = sizeof(u32);
> +	}
> +
> +	flush_icache_range((uintptr_t)(addr),
> +			   (uintptr_t)(addr) + size);
> +}
> +
> +static int __kprobes patch_text_stop_machine(void *data)
> +{
> +	struct patch *patch = data;
> +
> +	__patch_text(patch->addr, patch->insn);
> +
> +	return 0;
> +}
> +
> +void __kprobes patch_text(void *addr, unsigned int insn)
> +{
> +	struct patch patch = {
> +		.addr = addr,
> +		.insn = insn,
> +	};
> +
> +	if (cache_ops_need_broadcast()) {
> +		stop_machine(patch_text_stop_machine, &patch, cpu_online_mask);
> +	} else {
> +		bool straddles_word = IS_ENABLED(CONFIG_THUMB2_KERNEL)
> +				      && __opcode_is_thumb32(insn)
> +				      && ((uintptr_t)addr & 2);
> +
> +		if (straddles_word)
> +			stop_machine(patch_text_stop_machine, &patch, NULL);
> +		else
> +			__patch_text(addr, insn);
> +	}
> +}
> diff --git a/arch/arm/kernel/patch.h b/arch/arm/kernel/patch.h
> new file mode 100644
> index 0000000..b4731f2
> --- /dev/null
> +++ b/arch/arm/kernel/patch.h
> @@ -0,0 +1,7 @@
> +#ifndef _ARM_KERNEL_PATCH_H
> +#define _ARM_KERNEL_PATCH_H
> +
> +void patch_text(void *addr, unsigned int insn);
> +void __patch_text(void *addr, unsigned int insn);
> +
> +#endif

[PATCHv3] ARM: extract out code patch function from kprobes

[Rabin Vincent]

Extract out the code patching code from kprobes so that it can be used
from the jump label code.  Additionally, the separated code:

 - Uses the IS_ENABLED() macros instead of the #ifdefs for THUMB2
   support

 - Unifies the two separate functions in kprobes, providing one function
   that uses stop_machine() internally, and one that can be called from
   stop_machine() directly

 - Patches the text on all CPUs only on processors requiring software
   broadcasting of cache operations

Acked-by: Jon Medhurst <tixy@yxit.co.uk>
Tested-by: Jon Medhurst <tixy@yxit.co.uk>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
v3: add missing __opcode_to_mem_thumb16 for the split thumb2 case

 arch/arm/kernel/Makefile  |    3 +-
 arch/arm/kernel/kprobes.c |   86 ++++++++++++--------------------------------
 arch/arm/kernel/patch.c   |   75 +++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/patch.h   |    7 ++++
 4 files changed, 108 insertions(+), 63 deletions(-)
 create mode 100644 arch/arm/kernel/patch.c
 create mode 100644 arch/arm/kernel/patch.h

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index dd3d5f1..8c63ee8 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -8,6 +8,7 @@ AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg
+CFLAGS_REMOVE_patch.o = -pg
 endif
 
 CFLAGS_REMOVE_return_address.o = -pg
@@ -38,7 +39,7 @@ obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
-obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o
+obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o patch.o
 ifdef CONFIG_THUMB2_KERNEL
 obj-$(CONFIG_KPROBES)		+= kprobes-thumb.o
 else
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 129c116..ab1869d 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -29,6 +29,7 @@
 #include <asm/cacheflush.h>
 
 #include "kprobes.h"
+#include "patch.h"
 
 #define MIN_STACK_SIZE(addr) 				\
 	min((unsigned long)MAX_STACK_SIZE,		\
@@ -103,57 +104,33 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	return 0;
 }
 
-#ifdef CONFIG_THUMB2_KERNEL
-
-/*
- * For a 32-bit Thumb breakpoint spanning two memory words we need to take
- * special precautions to insert the breakpoint atomically, especially on SMP
- * systems. This is achieved by calling this arming function using stop_machine.
- */
-static int __kprobes set_t32_breakpoint(void *addr)
-{
-	((u16 *)addr)[0] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION >> 16;
-	((u16 *)addr)[1] = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION & 0xffff;
-	flush_insns(addr, 2*sizeof(u16));
-	return 0;
-}
-
 void __kprobes arch_arm_kprobe(struct kprobe *p)
 {
-	uintptr_t addr = (uintptr_t)p->addr & ~1; /* Remove any Thumb flag */
-
-	if (!is_wide_instruction(p->opcode)) {
-		*(u16 *)addr = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
-		flush_insns(addr, sizeof(u16));
-	} else if (addr & 2) {
-		/* A 32-bit instruction spanning two words needs special care */
-		stop_machine(set_t32_breakpoint, (void *)addr, &cpu_online_map);
+	unsigned int brkp;
+	void *addr;
+
+	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
+		/* Remove any Thumb flag */
+		addr = (void *)((uintptr_t)p->addr & ~1);
+
+		if (is_wide_instruction(p->opcode))
+			brkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
+		else
+			brkp = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
 	} else {
-		/* Word aligned 32-bit instruction can be written atomically */
-		u32 bkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
-#ifndef __ARMEB__ /* Swap halfwords for little-endian */
-		bkp = (bkp >> 16) | (bkp << 16);
-#endif
-		*(u32 *)addr = bkp;
-		flush_insns(addr, sizeof(u32));
-	}
-}
+		kprobe_opcode_t insn = p->opcode;
 
-#else /* !CONFIG_THUMB2_KERNEL */
+		addr = p->addr;
+		brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
 
-void __kprobes arch_arm_kprobe(struct kprobe *p)
-{
-	kprobe_opcode_t insn = p->opcode;
-	kprobe_opcode_t brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
-	if (insn >= 0xe0000000)
-		brkp |= 0xe0000000;  /* Unconditional instruction */
-	else
-		brkp |= insn & 0xf0000000;  /* Copy condition from insn */
-	*p->addr = brkp;
-	flush_insns(p->addr, sizeof(p->addr[0]));
-}
+		if (insn >= 0xe0000000)
+			brkp |= 0xe0000000;  /* Unconditional instruction */
+		else
+			brkp |= insn & 0xf0000000;  /* Copy condition from insn */
+	}
 
-#endif /* !CONFIG_THUMB2_KERNEL */
+	patch_text(addr, brkp);
+}
 
 /*
  * The actual disarming is done here on each CPU and synchronized using
@@ -166,25 +143,10 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
 int __kprobes __arch_disarm_kprobe(void *p)
 {
 	struct kprobe *kp = p;
-#ifdef CONFIG_THUMB2_KERNEL
-	u16 *addr = (u16 *)((uintptr_t)kp->addr & ~1);
-	kprobe_opcode_t insn = kp->opcode;
-	unsigned int len;
+	void *addr = (void *)((uintptr_t)kp->addr & ~1);
 
-	if (is_wide_instruction(insn)) {
-		((u16 *)addr)[0] = insn>>16;
-		((u16 *)addr)[1] = insn;
-		len = 2*sizeof(u16);
-	} else {
-		((u16 *)addr)[0] = insn;
-		len = sizeof(u16);
-	}
-	flush_insns(addr, len);
+	__patch_text(addr, kp->opcode);
 
-#else /* !CONFIG_THUMB2_KERNEL */
-	*kp->addr = kp->opcode;
-	flush_insns(kp->addr, sizeof(kp->addr[0]));
-#endif
 	return 0;
 }
 
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
new file mode 100644
index 0000000..07314af
--- /dev/null
+++ b/arch/arm/kernel/patch.c
@@ -0,0 +1,75 @@
+#include <linux/kernel.h>
+#include <linux/kprobes.h>
+#include <linux/stop_machine.h>
+
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/opcodes.h>
+
+#include "patch.h"
+
+struct patch {
+	void *addr;
+	unsigned int insn;
+};
+
+void __kprobes __patch_text(void *addr, unsigned int insn)
+{
+	bool thumb2 = IS_ENABLED(CONFIG_THUMB2_KERNEL);
+	int size;
+
+	if (thumb2 && __opcode_is_thumb16(insn)) {
+		*(u16 *)addr = __opcode_to_mem_thumb16(insn);
+		size = sizeof(u16);
+	} else if (thumb2 && ((uintptr_t)addr & 2)) {
+		u16 first = __opcode_thumb32_first(insn);
+		u16 second = __opcode_thumb32_second(insn);
+		u16 *addrh = addr;
+
+		addrh[0] = __opcode_to_mem_thumb16(first);
+		addrh[1] = __opcode_to_mem_thumb16(second);
+
+		size = sizeof(u32);
+	} else {
+		if (thumb2)
+			insn = __opcode_to_mem_thumb32(insn);
+		else
+			insn = __opcode_to_mem_arm(insn);
+
+		*(u32 *)addr = insn;
+		size = sizeof(u32);
+	}
+
+	flush_icache_range((uintptr_t)(addr),
+			   (uintptr_t)(addr) + size);
+}
+
+static int __kprobes patch_text_stop_machine(void *data)
+{
+	struct patch *patch = data;
+
+	__patch_text(patch->addr, patch->insn);
+
+	return 0;
+}
+
+void __kprobes patch_text(void *addr, unsigned int insn)
+{
+	struct patch patch = {
+		.addr = addr,
+		.insn = insn,
+	};
+
+	if (cache_ops_need_broadcast()) {
+		stop_machine(patch_text_stop_machine, &patch, cpu_online_mask);
+	} else {
+		bool straddles_word = IS_ENABLED(CONFIG_THUMB2_KERNEL)
+				      && __opcode_is_thumb32(insn)
+				      && ((uintptr_t)addr & 2);
+
+		if (straddles_word)
+			stop_machine(patch_text_stop_machine, &patch, NULL);
+		else
+			__patch_text(addr, insn);
+	}
+}
diff --git a/arch/arm/kernel/patch.h b/arch/arm/kernel/patch.h
new file mode 100644
index 0000000..b4731f2
--- /dev/null
+++ b/arch/arm/kernel/patch.h
@@ -0,0 +1,7 @@
+#ifndef _ARM_KERNEL_PATCH_H
+#define _ARM_KERNEL_PATCH_H
+
+void patch_text(void *addr, unsigned int insn);
+void __patch_text(void *addr, unsigned int insn);
+
+#endif
-- 
1.7.9

Re: [PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

Jason,

On Sat, Jan 28, 2012 at 19:05, Rabin Vincent <rabin@rab.in> wrote:
> Some versions of ARM GCC which do support asm goto, have problems
> handling the the %c specifier.  Since we need the %c to support jump
> labels on ARM, detect that too in the asm goto detection script to avoid
> build errors with these versions.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Rabin Vincent <rabin@rab.in>

Could you please ack this patch if it's OK for you?  Then I could try to
send it via rmk's ARM patch system along with the rest of the series.

Thanks.

> ---
>  scripts/gcc-goto.sh |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> index 98cffcb..9b744de 100644
> --- a/scripts/gcc-goto.sh
> +++ b/scripts/gcc-goto.sh
> @@ -2,4 +2,20 @@
>  # Test for gcc 'asm goto' support
>  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
>
> -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +int main(void)
> +{
> +#ifdef __arm__
> +       /*
> +        * Not related to asm goto, but used by jump label
> +        * and broken on some ARM GCC versions (see GCC Bug 48637).
> +        */
> +       static struct { int dummy; int state; } tp;
> +       asm ("@ %c0" :: "i" (&tp.state));
> +#endif
> +
> +entry:
> +       asm goto ("" :::: entry);
> +       return 0;
> +}
> +END
> --
> 1.7.8.3
>

[PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

Jason,

On Sat, Jan 28, 2012 at 19:05, Rabin Vincent <rabin@rab.in> wrote:
> Some versions of ARM GCC which do support asm goto, have problems
> handling the the %c specifier. ?Since we need the %c to support jump
> labels on ARM, detect that too in the asm goto detection script to avoid
> build errors with these versions.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Rabin Vincent <rabin@rab.in>

Could you please ack this patch if it's OK for you?  Then I could try to
send it via rmk's ARM patch system along with the rest of the series.

Thanks.

> ---
> ?scripts/gcc-goto.sh | ? 18 +++++++++++++++++-
> ?1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> index 98cffcb..9b744de 100644
> --- a/scripts/gcc-goto.sh
> +++ b/scripts/gcc-goto.sh
> @@ -2,4 +2,20 @@
> ?# Test for gcc 'asm goto' support
> ?# Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
>
> -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +int main(void)
> +{
> +#ifdef __arm__
> + ? ? ? /*
> + ? ? ? ?* Not related to asm goto, but used by jump label
> + ? ? ? ?* and broken on some ARM GCC versions (see GCC Bug 48637).
> + ? ? ? ?*/
> + ? ? ? static struct { int dummy; int state; } tp;
> + ? ? ? asm ("@ %c0" :: "i" (&tp.state));
> +#endif
> +
> +entry:
> + ? ? ? asm goto ("" :::: entry);
> + ? ? ? return 0;
> +}
> +END
> --
> 1.7.8.3
>

Re: [PATCHv2 5/6] jump label: detect %c support for ARM

[Jason Baron]

On Tue, Feb 07, 2012 at 09:48:24PM +0530, Rabin Vincent wrote:
> Jason,
> 
> On Sat, Jan 28, 2012 at 19:05, Rabin Vincent <rabin@rab.in> wrote:
> > Some versions of ARM GCC which do support asm goto, have problems
> > handling the the %c specifier.  Since we need the %c to support jump
> > labels on ARM, detect that too in the asm goto detection script to avoid
> > build errors with these versions.
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> >
> > Cc: Jason Baron <jbaron@redhat.com>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> 
> Could you please ack this patch if it's OK for you?  Then I could try to
> send it via rmk's ARM patch system along with the rest of the series.
> 
> Thanks.
> 
> > ---
> >  scripts/gcc-goto.sh |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> > index 98cffcb..9b744de 100644
> > --- a/scripts/gcc-goto.sh
> > +++ b/scripts/gcc-goto.sh
> > @@ -2,4 +2,20 @@
> >  # Test for gcc 'asm goto' support
> >  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
> >
> > -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +int main(void)
> > +{
> > +#ifdef __arm__
> > +       /*
> > +        * Not related to asm goto, but used by jump label
> > +        * and broken on some ARM GCC versions (see GCC Bug 48637).
> > +        */
> > +       static struct { int dummy; int state; } tp;
> > +       asm ("@ %c0" :: "i" (&tp.state));
> > +#endif
> > +
> > +entry:
> > +       asm goto ("" :::: entry);
> > +       return 0;
> > +}
> > +END
> > --
> > 1.7.8.3
> >

Sure, patch works for me.

Acked-by: Jason Baron <jbaron@redhat.com>

[PATCHv2 5/6] jump label: detect %c support for ARM

[Jason Baron]

On Tue, Feb 07, 2012 at 09:48:24PM +0530, Rabin Vincent wrote:
> Jason,
> 
> On Sat, Jan 28, 2012 at 19:05, Rabin Vincent <rabin@rab.in> wrote:
> > Some versions of ARM GCC which do support asm goto, have problems
> > handling the the %c specifier. ?Since we need the %c to support jump
> > labels on ARM, detect that too in the asm goto detection script to avoid
> > build errors with these versions.
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> >
> > Cc: Jason Baron <jbaron@redhat.com>
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> 
> Could you please ack this patch if it's OK for you?  Then I could try to
> send it via rmk's ARM patch system along with the rest of the series.
> 
> Thanks.
> 
> > ---
> > ?scripts/gcc-goto.sh | ? 18 +++++++++++++++++-
> > ?1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> > index 98cffcb..9b744de 100644
> > --- a/scripts/gcc-goto.sh
> > +++ b/scripts/gcc-goto.sh
> > @@ -2,4 +2,20 @@
> > ?# Test for gcc 'asm goto' support
> > ?# Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
> >
> > -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +int main(void)
> > +{
> > +#ifdef __arm__
> > + ? ? ? /*
> > + ? ? ? ?* Not related to asm goto, but used by jump label
> > + ? ? ? ?* and broken on some ARM GCC versions (see GCC Bug 48637).
> > + ? ? ? ?*/
> > + ? ? ? static struct { int dummy; int state; } tp;
> > + ? ? ? asm ("@ %c0" :: "i" (&tp.state));
> > +#endif
> > +
> > +entry:
> > + ? ? ? asm goto ("" :::: entry);
> > + ? ? ? return 0;
> > +}
> > +END
> > --
> > 1.7.8.3
> >

Sure, patch works for me.

Acked-by: Jason Baron <jbaron@redhat.com>

[PATCHv3] ARM: add jump label support

[Rabin Vincent]

Add the arch-specific code to support jump labels for ARM and Thumb-2.

This code will only be activated on compilers that are capable of
building it.  It has been tested with GCC 4.6 patched with the patch
from GCC bug 48637.

Cc: Jason Baron <jbaron@redhat.com>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
v3: add missing #ifdef HAVE_JUMP_LABEL around the code in the C file

 arch/arm/Kconfig                  |    1 +
 arch/arm/include/asm/jump_label.h |   41 +++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/Makefile          |    1 +
 arch/arm/kernel/insn.h            |   10 +++++++++
 arch/arm/kernel/jump_label.c      |   39 +++++++++++++++++++++++++++++++++++
 5 files changed, 92 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/jump_label.h
 create mode 100644 arch/arm/kernel/jump_label.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 57b6e96..aaf9a3c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -9,6 +9,7 @@ config ARM
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_ATOMIC64 if (CPU_V6 || !CPU_32v6K || !AEABI)
 	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
+	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
 	select HAVE_ARCH_KGDB
 	select HAVE_KPROBES if !XIP_KERNEL
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
new file mode 100644
index 0000000..5c5ca2e
--- /dev/null
+++ b/arch/arm/include/asm/jump_label.h
@@ -0,0 +1,41 @@
+#ifndef _ASM_ARM_JUMP_LABEL_H
+#define _ASM_ARM_JUMP_LABEL_H
+
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <asm/system.h>
+
+#define JUMP_LABEL_NOP_SIZE 4
+
+#ifdef CONFIG_THUMB2_KERNEL
+#define JUMP_LABEL_NOP	"nop.w"
+#else
+#define JUMP_LABEL_NOP	"nop"
+#endif
+
+static __always_inline bool arch_static_branch(struct jump_label_key *key)
+{
+	asm goto("1:\n\t"
+		 JUMP_LABEL_NOP "\n\t"
+		 ".pushsection __jump_table,  \"aw\"\n\t"
+		 ".word 1b, %l[l_yes], %c0\n\t"
+		 ".popsection\n\t"
+		 : :  "i" (key) :  : l_yes);
+
+	return false;
+l_yes:
+	return true;
+}
+
+#endif /* __KERNEL__ */
+
+typedef u32 jump_label_t;
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
+#endif
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 8c63ee8..1b7d9a3 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_HAVE_ARM_SCU)	+= smp_scu.o
 obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o insn.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
+obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o insn.o patch.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes-common.o patch.o
 ifdef CONFIG_THUMB2_KERNEL
diff --git a/arch/arm/kernel/insn.h b/arch/arm/kernel/insn.h
index 994d60f..e96065d 100644
--- a/arch/arm/kernel/insn.h
+++ b/arch/arm/kernel/insn.h
@@ -1,6 +1,16 @@
 #ifndef __ASM_ARM_INSN_H
 #define __ASM_ARM_INSN_H
 
+static inline unsigned long
+arm_gen_nop(void)
+{
+#ifdef CONFIG_THUMB2_KERNEL
+	return 0xf3af8000; /* nop.w */
+#else
+	return 0xe1a00000; /* mov r0, r0 */
+#endif
+}
+
 unsigned long
 __arm_gen_branch(unsigned long pc, unsigned long addr, bool link);
 
diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c
new file mode 100644
index 0000000..4ce4f78
--- /dev/null
+++ b/arch/arm/kernel/jump_label.c
@@ -0,0 +1,39 @@
+#include <linux/kernel.h>
+#include <linux/jump_label.h>
+
+#include "insn.h"
+#include "patch.h"
+
+#ifdef HAVE_JUMP_LABEL
+
+static void __arch_jump_label_transform(struct jump_entry *entry,
+					enum jump_label_type type,
+					bool is_static)
+{
+	void *addr = (void *)entry->code;
+	unsigned int insn;
+
+	if (type == JUMP_LABEL_ENABLE)
+		insn = arm_gen_branch(entry->code, entry->target);
+	else
+		insn = arm_gen_nop();
+
+	if (is_static)
+		__patch_text(addr, insn);
+	else
+		patch_text(addr, insn);
+}
+
+void arch_jump_label_transform(struct jump_entry *entry,
+			       enum jump_label_type type)
+{
+	__arch_jump_label_transform(entry, type, false);
+}
+
+void arch_jump_label_transform_static(struct jump_entry *entry,
+				      enum jump_label_type type)
+{
+	__arch_jump_label_transform(entry, type, true);
+}
+
+#endif
-- 
1.7.9

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Russell King - ARM Linux]

On Sat, Jan 28, 2012 at 07:05:20PM +0530, Rabin Vincent wrote:
> Before replacing an instruction, the ftrace code determines what the old
> instruction should be and verifies that that's what's really there in
> memory before replacing it.  This is useful if for example a bug in
> mcountrecord causes it to record wrong locations.
> 
> However, in cases where we replace call sites in entry-common.S, these
> checks are not needed.  For these, we currently just memcpy() the memory
> content and then "verify" it -- this is quite useless and can be
> removed.

Yes, I know this is a little late, but...

> 
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  arch/arm/kernel/ftrace.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index c0062ad..e9488ad 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -125,11 +125,13 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
>  {
>  	unsigned long replaced;
>  
> -	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
> -		return -EFAULT;
> +	if (old) {

So, we're using the instruction value '0' to mean that we don't want to
check?  Wouldn'it it be better to pass a flag in to indicate this instead
of creating a magic value?

Re: [PATCHv2 5/6] jump label: detect %c support for ARM

[Russell King - ARM Linux]

On Sat, Jan 28, 2012 at 07:05:24PM +0530, Rabin Vincent wrote:
> Some versions of ARM GCC which do support asm goto, have problems
> handling the the %c specifier.  Since we need the %c to support jump
> labels on ARM, detect that too in the asm goto detection script to avoid
> build errors with these versions.

I'm not sure how this detects properly working %c support.  %c support
has been rather flakey on ARM for some time, I'm not sure which version
of gcc was fixed for it.

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> 
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  scripts/gcc-goto.sh |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> index 98cffcb..9b744de 100644
> --- a/scripts/gcc-goto.sh
> +++ b/scripts/gcc-goto.sh
> @@ -2,4 +2,20 @@
>  # Test for gcc 'asm goto' support
>  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
>  
> -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +int main(void)
> +{
> +#ifdef __arm__
> +	/*
> +	 * Not related to asm goto, but used by jump label
> +	 * and broken on some ARM GCC versions (see GCC Bug 48637).
> +	 */
> +	static struct { int dummy; int state; } tp;
> +	asm ("@ %c0" :: "i" (&tp.state));

While this detects the presence of the ICE, previous compilers included
the '#' before the constant, which leads to problems.  I think a better
check would be:
	asm(".long %c0" :: "i" (&tp.state));

to make sure that it spits out something purely numeric, rather than
something prefixed with a '#'.

[PATCHv2 5/6] jump label: detect %c support for ARM

[Russell King - ARM Linux]

On Sat, Jan 28, 2012 at 07:05:24PM +0530, Rabin Vincent wrote:
> Some versions of ARM GCC which do support asm goto, have problems
> handling the the %c specifier.  Since we need the %c to support jump
> labels on ARM, detect that too in the asm goto detection script to avoid
> build errors with these versions.

I'm not sure how this detects properly working %c support.  %c support
has been rather flakey on ARM for some time, I'm not sure which version
of gcc was fixed for it.

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> 
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  scripts/gcc-goto.sh |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> index 98cffcb..9b744de 100644
> --- a/scripts/gcc-goto.sh
> +++ b/scripts/gcc-goto.sh
> @@ -2,4 +2,20 @@
>  # Test for gcc 'asm goto' support
>  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
>  
> -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> +int main(void)
> +{
> +#ifdef __arm__
> +	/*
> +	 * Not related to asm goto, but used by jump label
> +	 * and broken on some ARM GCC versions (see GCC Bug 48637).
> +	 */
> +	static struct { int dummy; int state; } tp;
> +	asm ("@ %c0" :: "i" (&tp.state));

While this detects the presence of the ICE, previous compilers included
the '#' before the constant, which leads to problems.  I think a better
check would be:
	asm(".long %c0" :: "i" (&tp.state));

to make sure that it spits out something purely numeric, rather than
something prefixed with a '#'.

Re: [PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

On Mon, Feb 20, 2012 at 05:21:58PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 28, 2012 at 07:05:24PM +0530, Rabin Vincent wrote:
> > Some versions of ARM GCC which do support asm goto, have problems
> > handling the the %c specifier.  Since we need the %c to support jump
> > labels on ARM, detect that too in the asm goto detection script to avoid
> > build errors with these versions.
> 
> I'm not sure how this detects properly working %c support.  %c support
> has been rather flakey on ARM for some time, I'm not sure which version
> of gcc was fixed for it.

Right, it only detects that %c on a sym+offset doesn't ICE, which was
what it had been doing since atleast version 4.3.

The assumption was that since asm goto was only added in ~4.5, all older
compilers (regardless of their state of %c support) would choke (as we
want them to) on the existing second half of this script.

I have added the change you suggested below.

> 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> > 
> > Cc: Jason Baron <jbaron@redhat.com>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> > ---
> >  scripts/gcc-goto.sh |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> > 
> > diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> > index 98cffcb..9b744de 100644
> > --- a/scripts/gcc-goto.sh
> > +++ b/scripts/gcc-goto.sh
> > @@ -2,4 +2,20 @@
> >  # Test for gcc 'asm goto' support
> >  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
> >  
> > -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +int main(void)
> > +{
> > +#ifdef __arm__
> > +	/*
> > +	 * Not related to asm goto, but used by jump label
> > +	 * and broken on some ARM GCC versions (see GCC Bug 48637).
> > +	 */
> > +	static struct { int dummy; int state; } tp;
> > +	asm ("@ %c0" :: "i" (&tp.state));
> 
> While this detects the presence of the ICE, previous compilers included
> the '#' before the constant, which leads to problems.  I think a better
> check would be:
> 	asm(".long %c0" :: "i" (&tp.state));
> 
> to make sure that it spits out something purely numeric, rather than
> something prefixed with a '#'.

OK, done:

8<----------------
>From f3a3bd64f72720def5cfbaedf6ff83ca54f1644b Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Wed, 23 Nov 2011 20:05:09 +0530
Subject: [PATCH] jump label: detect %c support for ARM

Some versions of ARM GCC which do support asm goto, do not support
the %c specifier.  Since we need the %c to support jump labels
on ARM, detect that too in the asm goto detection script to avoid
build errors with these versions.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637

Acked-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 scripts/gcc-goto.sh |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
index 98cffcb..a2af2e8 100644
--- a/scripts/gcc-goto.sh
+++ b/scripts/gcc-goto.sh
@@ -2,4 +2,20 @@
 # Test for gcc 'asm goto' support
 # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
 
-echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+int main(void)
+{
+#ifdef __arm__
+	/*
+	 * Not related to asm goto, but used by jump label
+	 * and broken on some ARM GCC versions (see GCC Bug 48637).
+	 */
+	static struct { int dummy; int state; } tp;
+	asm (".long %c0" :: "i" (&tp.state));
+#endif
+
+entry:
+	asm goto ("" :::: entry);
+	return 0;
+}
+END
-- 
1.7.9

[PATCHv2 5/6] jump label: detect %c support for ARM

[Rabin Vincent]

On Mon, Feb 20, 2012 at 05:21:58PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 28, 2012 at 07:05:24PM +0530, Rabin Vincent wrote:
> > Some versions of ARM GCC which do support asm goto, have problems
> > handling the the %c specifier.  Since we need the %c to support jump
> > labels on ARM, detect that too in the asm goto detection script to avoid
> > build errors with these versions.
> 
> I'm not sure how this detects properly working %c support.  %c support
> has been rather flakey on ARM for some time, I'm not sure which version
> of gcc was fixed for it.

Right, it only detects that %c on a sym+offset doesn't ICE, which was
what it had been doing since atleast version 4.3.

The assumption was that since asm goto was only added in ~4.5, all older
compilers (regardless of their state of %c support) would choke (as we
want them to) on the existing second half of this script.

I have added the change you suggested below.

> 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637
> > 
> > Cc: Jason Baron <jbaron@redhat.com>
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> > ---
> >  scripts/gcc-goto.sh |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> > 
> > diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
> > index 98cffcb..9b744de 100644
> > --- a/scripts/gcc-goto.sh
> > +++ b/scripts/gcc-goto.sh
> > @@ -2,4 +2,20 @@
> >  # Test for gcc 'asm goto' support
> >  # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
> >  
> > -echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
> > +int main(void)
> > +{
> > +#ifdef __arm__
> > +	/*
> > +	 * Not related to asm goto, but used by jump label
> > +	 * and broken on some ARM GCC versions (see GCC Bug 48637).
> > +	 */
> > +	static struct { int dummy; int state; } tp;
> > +	asm ("@ %c0" :: "i" (&tp.state));
> 
> While this detects the presence of the ICE, previous compilers included
> the '#' before the constant, which leads to problems.  I think a better
> check would be:
> 	asm(".long %c0" :: "i" (&tp.state));
> 
> to make sure that it spits out something purely numeric, rather than
> something prefixed with a '#'.

OK, done:

8<----------------
>From f3a3bd64f72720def5cfbaedf6ff83ca54f1644b Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Wed, 23 Nov 2011 20:05:09 +0530
Subject: [PATCH] jump label: detect %c support for ARM

Some versions of ARM GCC which do support asm goto, do not support
the %c specifier.  Since we need the %c to support jump labels
on ARM, detect that too in the asm goto detection script to avoid
build errors with these versions.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48637

Acked-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 scripts/gcc-goto.sh |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh
index 98cffcb..a2af2e8 100644
--- a/scripts/gcc-goto.sh
+++ b/scripts/gcc-goto.sh
@@ -2,4 +2,20 @@
 # Test for gcc 'asm goto' support
 # Copyright (C) 2010, Jason Baron <jbaron@redhat.com>
 
-echo "int main(void) { entry: asm goto (\"\"::::entry); return 0; }" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y"
+int main(void)
+{
+#ifdef __arm__
+	/*
+	 * Not related to asm goto, but used by jump label
+	 * and broken on some ARM GCC versions (see GCC Bug 48637).
+	 */
+	static struct { int dummy; int state; } tp;
+	asm (".long %c0" :: "i" (&tp.state));
+#endif
+
+entry:
+	asm goto ("" :::: entry);
+	return 0;
+}
+END
-- 
1.7.9

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Rabin Vincent]

On Mon, Feb 20, 2012 at 04:16:01PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 28, 2012 at 07:05:20PM +0530, Rabin Vincent wrote:
> > diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> > index c0062ad..e9488ad 100644
> > --- a/arch/arm/kernel/ftrace.c
> > +++ b/arch/arm/kernel/ftrace.c
> > @@ -125,11 +125,13 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
> >  {
> >  	unsigned long replaced;
> >  
> > -	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
> > -		return -EFAULT;
> > +	if (old) {
> 
> So, we're using the instruction value '0' to mean that we don't want to
> check?  Wouldn'it it be better to pass a flag in to indicate this instead
> of creating a magic value?

OK.  I think you applied this patch as-is anyway, so here is a follow-on
patch:

8<------------
>From 18ad9e696a3b3986babb1c91807d3f39d6468176 Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Wed, 22 Feb 2012 19:11:46 +0530
Subject: [PATCH] ARM: ftrace: use flag for memory check

We currently use a (magic) value of 0 for the old instruction value
to detect if we should verify the old value in memory or not.  Pass
a flag instead.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/ftrace.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 858c63f..df0bf0c 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -69,7 +69,7 @@ static unsigned long ftrace_call_replace(unsigned long pc, unsigned long addr)
 }
 
 static int ftrace_modify_code(unsigned long pc, unsigned long old,
-			      unsigned long new)
+			      unsigned long new, bool validate)
 {
 	unsigned long replaced;
 
@@ -81,7 +81,7 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 		new = __opcode_to_mem_arm(new);
 	}
 
-	if (old) {
+	if (validate) {
 		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
 			return -EFAULT;
 
@@ -106,14 +106,14 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	pc = (unsigned long)&ftrace_call;
 	new = ftrace_call_replace(pc, (unsigned long)func);
 
-	ret = ftrace_modify_code(pc, 0, new);
+	ret = ftrace_modify_code(pc, 0, new, false);
 
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
 		new = ftrace_call_replace(pc, (unsigned long)func);
 
-		ret = ftrace_modify_code(pc, 0, new);
+		ret = ftrace_modify_code(pc, 0, new, false);
 	}
 #endif
 
@@ -128,7 +128,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	old = ftrace_nop_replace(rec);
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
-	return ftrace_modify_code(rec->ip, old, new);
+	return ftrace_modify_code(rec->ip, old, new, true);
 }
 
 int ftrace_make_nop(struct module *mod,
@@ -141,7 +141,7 @@ int ftrace_make_nop(struct module *mod,
 
 	old = ftrace_call_replace(ip, adjust_address(rec, addr));
 	new = ftrace_nop_replace(rec);
-	ret = ftrace_modify_code(ip, old, new);
+	ret = ftrace_modify_code(ip, old, new, true);
 
 #ifdef CONFIG_OLD_MCOUNT
 	if (ret == -EINVAL && addr == MCOUNT_ADDR) {
@@ -149,7 +149,7 @@ int ftrace_make_nop(struct module *mod,
 
 		old = ftrace_call_replace(ip, adjust_address(rec, addr));
 		new = ftrace_nop_replace(rec);
-		ret = ftrace_modify_code(ip, old, new);
+		ret = ftrace_modify_code(ip, old, new, true);
 	}
 #endif
 
@@ -210,7 +210,7 @@ static int __ftrace_modify_caller(unsigned long *callsite,
 	unsigned long old = enable ? nop : branch;
 	unsigned long new = enable ? branch : nop;
 
-	return ftrace_modify_code(pc, old, new);
+	return ftrace_modify_code(pc, old, new, true);
 }
 
 static int ftrace_modify_graph_caller(bool enable)
-- 
1.7.9

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Russell King - ARM Linux]

On Wed, Feb 22, 2012 at 07:43:17PM +0530, Rabin Vincent wrote:
> On Mon, Feb 20, 2012 at 04:16:01PM +0000, Russell King - ARM Linux wrote:
> > On Sat, Jan 28, 2012 at 07:05:20PM +0530, Rabin Vincent wrote:
> > > diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> > > index c0062ad..e9488ad 100644
> > > --- a/arch/arm/kernel/ftrace.c
> > > +++ b/arch/arm/kernel/ftrace.c
> > > @@ -125,11 +125,13 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
> > >  {
> > >  	unsigned long replaced;
> > >  
> > > -	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
> > > -		return -EFAULT;
> > > +	if (old) {
> > 
> > So, we're using the instruction value '0' to mean that we don't want to
> > check?  Wouldn'it it be better to pass a flag in to indicate this instead
> > of creating a magic value?
> 
> OK.  I think you applied this patch as-is anyway, so here is a follow-on
> patch:

I'd still much prefer it to be part of the original patch.  I can replace
the patch I've merged or augment it with another patch - whichever way
you prefer.

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Rabin Vincent]

On Wed, Feb 22, 2012 at 09:28:21PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 22, 2012 at 07:43:17PM +0530, Rabin Vincent wrote:
> > On Mon, Feb 20, 2012 at 04:16:01PM +0000, Russell King - ARM Linux wrote:
> > > On Sat, Jan 28, 2012 at 07:05:20PM +0530, Rabin Vincent wrote:
> > > > diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> > > > index c0062ad..e9488ad 100644
> > > > --- a/arch/arm/kernel/ftrace.c
> > > > +++ b/arch/arm/kernel/ftrace.c
> > > > @@ -125,11 +125,13 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
> > > >  {
> > > >  	unsigned long replaced;
> > > >  
> > > > -	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
> > > > -		return -EFAULT;
> > > > +	if (old) {
> > > 
> > > So, we're using the instruction value '0' to mean that we don't want to
> > > check?  Wouldn'it it be better to pass a flag in to indicate this instead
> > > of creating a magic value?
> > 
> > OK.  I think you applied this patch as-is anyway, so here is a follow-on
> > patch:
> 
> I'd still much prefer it to be part of the original patch.  I can replace
> the patch I've merged or augment it with another patch - whichever way
> you prefer.

Replacing the original is OK with me, new patch below.  Note that "ARM:
ftrace: use canonical Thumb-2 wide instruction format" will need to be
manually rebased on top of it since there is a change in the context.

8<----------------
>From 11a4aea10f1b2d56c70c34def29b3f8d56b88dd5 Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin@rab.in>
Date: Sat, 21 Jan 2012 21:52:19 +0530
Subject: [PATCH] ARM: ftrace: remove useless memory checks

Before replacing an instruction, the ftrace code determines what the old
instruction should be and verifies that that's what's really there in
memory before replacing it.  This is useful if for example a bug in
mcountrecord causes it to record wrong locations.

However, in cases where we replace call sites in entry-common.S, these
checks are not needed.  For these, we currently just memcpy() the memory
content and then "verify" it -- this is quite useless and can be
removed.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 arch/arm/kernel/ftrace.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index c0062ad..6fd7c4a 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -121,15 +121,17 @@ static unsigned long ftrace_call_replace(unsigned long pc, unsigned long addr)
 }
 
 static int ftrace_modify_code(unsigned long pc, unsigned long old,
-			      unsigned long new)
+			      unsigned long new, bool validate)
 {
 	unsigned long replaced;
 
-	if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
-		return -EFAULT;
+	if (validate) {
+		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
+			return -EFAULT;
 
-	if (replaced != old)
-		return -EINVAL;
+		if (replaced != old)
+			return -EINVAL;
+	}
 
 	if (probe_kernel_write((void *)pc, &new, MCOUNT_INSN_SIZE))
 		return -EPERM;
@@ -141,23 +143,21 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
-	unsigned long pc, old;
+	unsigned long pc;
 	unsigned long new;
 	int ret;
 
 	pc = (unsigned long)&ftrace_call;
-	memcpy(&old, &ftrace_call, MCOUNT_INSN_SIZE);
 	new = ftrace_call_replace(pc, (unsigned long)func);
 
-	ret = ftrace_modify_code(pc, old, new);
+	ret = ftrace_modify_code(pc, 0, new, false);
 
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
-		memcpy(&old, &ftrace_call_old, MCOUNT_INSN_SIZE);
 		new = ftrace_call_replace(pc, (unsigned long)func);
 
-		ret = ftrace_modify_code(pc, old, new);
+		ret = ftrace_modify_code(pc, 0, new, false);
 	}
 #endif
 
@@ -172,7 +172,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	old = ftrace_nop_replace(rec);
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
-	return ftrace_modify_code(rec->ip, old, new);
+	return ftrace_modify_code(rec->ip, old, new, true);
 }
 
 int ftrace_make_nop(struct module *mod,
@@ -185,7 +185,7 @@ int ftrace_make_nop(struct module *mod,
 
 	old = ftrace_call_replace(ip, adjust_address(rec, addr));
 	new = ftrace_nop_replace(rec);
-	ret = ftrace_modify_code(ip, old, new);
+	ret = ftrace_modify_code(ip, old, new, true);
 
 #ifdef CONFIG_OLD_MCOUNT
 	if (ret == -EINVAL && addr == MCOUNT_ADDR) {
@@ -193,7 +193,7 @@ int ftrace_make_nop(struct module *mod,
 
 		old = ftrace_call_replace(ip, adjust_address(rec, addr));
 		new = ftrace_nop_replace(rec);
-		ret = ftrace_modify_code(ip, old, new);
+		ret = ftrace_modify_code(ip, old, new, true);
 	}
 #endif
 
@@ -254,7 +254,7 @@ static int __ftrace_modify_caller(unsigned long *callsite,
 	unsigned long old = enable ? nop : branch;
 	unsigned long new = enable ? branch : nop;
 
-	return ftrace_modify_code(pc, old, new);
+	return ftrace_modify_code(pc, old, new, true);
 }
 
 static int ftrace_modify_graph_caller(bool enable)
-- 
1.7.9

[PATCHv2 1/6] ARM: ftrace: remove useless memory checks

[Russell King - ARM Linux]

On Fri, Feb 24, 2012 at 10:18:21PM +0530, Rabin Vincent wrote:
> Replacing the original is OK with me, new patch below.  Note that "ARM:
> ftrace: use canonical Thumb-2 wide instruction format" will need to be
> manually rebased on top of it since there is a change in the context.

Ok, please put this in the patch system, thanks.

[PATCHv3] ARM: add jump label support

[Rabin Vincent]

On Wed, Feb 15, 2012 at 10:30:44PM +0530, Rabin Vincent wrote:
> Add the arch-specific code to support jump labels for ARM and Thumb-2.
> 
> This code will only be activated on compilers that are capable of
> building it.  It has been tested with GCC 4.6 patched with the patch
> from GCC bug 48637.
> 
> Cc: Jason Baron <jbaron@redhat.com>
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
...
> +static __always_inline bool arch_static_branch(struct jump_label_key *key)

Hmm, there's now a patch in linux-next ("static keys: Introduce 'struct
static_key', static_key_true()/false() and static_key_slow_[inc|dec]()")
via the -tip tree which would cause this not to build anymore since
struct jump_label_key was renamed to struct static_key.

I'm not sure how this could be handled to keep this compiling both in
the arm tree separately and in linux-next.  I guess the easiest is to
delay this until the next cycle, since it's not applied yet.

[PATCHv3] ARM: add jump label support

[Jason Baron]

On Wed, Feb 29, 2012 at 08:54:06PM +0530, Rabin Vincent wrote:
> On Wed, Feb 15, 2012 at 10:30:44PM +0530, Rabin Vincent wrote:
> > Add the arch-specific code to support jump labels for ARM and Thumb-2.
> > 
> > This code will only be activated on compilers that are capable of
> > building it.  It has been tested with GCC 4.6 patched with the patch
> > from GCC bug 48637.
> > 
> > Cc: Jason Baron <jbaron@redhat.com>
> > Signed-off-by: Rabin Vincent <rabin@rab.in>
> > ---
> ...
> > +static __always_inline bool arch_static_branch(struct jump_label_key *key)
> 
> Hmm, there's now a patch in linux-next ("static keys: Introduce 'struct
> static_key', static_key_true()/false() and static_key_slow_[inc|dec]()")
> via the -tip tree which would cause this not to build anymore since
> struct jump_label_key was renamed to struct static_key.
> 
> I'm not sure how this could be handled to keep this compiling both in
> the arm tree separately and in linux-next.  I guess the easiest is to
> delay this until the next cycle, since it's not applied yet.

hmm...Maybe the rename patches could be pulled into the arm tree
somehow? Waiting a cycle works too...

Thanks,

-Jason