[PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[Michael Ellerman]

We currently have a few routines for patching code in asm/system.h, because
they didn't fit anywhere else. I'd like to clean them up a little and add
some more, so first move them into a dedicated C file - they don't need to
be inlined.

While we're moving the code, drop create_function_call(), it's intended
caller never got merged and will be replaced in future with something
different.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/crash_dump.c          |    1 +
 arch/powerpc/lib/Makefile                 |    2 +
 arch/powerpc/lib/code-patching.c          |   33 ++++++++++++++++++++
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |    1 +
 arch/powerpc/platforms/powermac/smp.c     |    1 +
 include/asm-powerpc/code-patching.h       |   25 +++++++++++++++
 include/asm-powerpc/system.h              |   48 -----------------------------
 7 files changed, 63 insertions(+), 48 deletions(-)

diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index 9ee3c52..35b9a66 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -14,6 +14,7 @@
 #include <linux/crash_dump.h>
 #include <linux/bootmem.h>
 #include <linux/lmb.h>
+#include <asm/code-patching.h>
 #include <asm/kdump.h>
 #include <asm/prom.h>
 #include <asm/firmware.h>
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index c71d37d..305c7df 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -24,3 +24,5 @@ obj-$(CONFIG_SMP)	+= locks.o
 endif
 
 obj-$(CONFIG_PPC_LIB_RHEAP) += rheap.o
+
+obj-y			+= code-patching.o
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
new file mode 100644
index 0000000..7afae88
--- /dev/null
+++ b/arch/powerpc/lib/code-patching.c
@@ -0,0 +1,33 @@
+/*
+ *  Copyright 2008 Michael Ellerman, IBM Corporation.
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <asm/code-patching.h>
+
+
+void create_instruction(unsigned long addr, unsigned int instr)
+{
+	unsigned int *p;
+	p  = (unsigned int *)addr;
+	*p = instr;
+	asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (p));
+}
+
+void create_branch(unsigned long addr, unsigned long target, int flags)
+{
+	unsigned int instruction;
+
+	if (! (flags & BRANCH_ABSOLUTE))
+		target = target - addr;
+
+	/* Mask out the flags and target, so they don't step on each other. */
+	instruction = 0x48000000 | (flags & 0x3) | (target & 0x03FFFFFC);
+
+	create_instruction(addr, instruction);
+}
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
index ba55b0f..63f5585 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 
+#include <asm/code-patching.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/pci-bridge.h>
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index cb2d894..bf202f7 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -36,6 +36,7 @@
 
 #include <asm/ptrace.h>
 #include <asm/atomic.h>
+#include <asm/code-patching.h>
 #include <asm/irq.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
diff --git a/include/asm-powerpc/code-patching.h b/include/asm-powerpc/code-patching.h
new file mode 100644
index 0000000..0b91fdf
--- /dev/null
+++ b/include/asm-powerpc/code-patching.h
@@ -0,0 +1,25 @@
+#ifndef _ASM_POWERPC_CODE_PATCHING_H
+#define _ASM_POWERPC_CODE_PATCHING_H
+
+/*
+ * Copyright 2008, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+/* Flags for create_branch:
+ * "b"   == create_branch(addr, target, 0);
+ * "ba"  == create_branch(addr, target, BRANCH_ABSOLUTE);
+ * "bl"  == create_branch(addr, target, BRANCH_SET_LINK);
+ * "bla" == create_branch(addr, target, BRANCH_ABSOLUTE | BRANCH_SET_LINK);
+ */
+#define BRANCH_SET_LINK	0x1
+#define BRANCH_ABSOLUTE	0x2
+
+extern void create_branch(unsigned long addr, unsigned long target, int flags);
+extern void create_instruction(unsigned long addr, unsigned int instr);
+
+#endif /* _ASM_POWERPC_CODE_PATCHING_H */
diff --git a/include/asm-powerpc/system.h b/include/asm-powerpc/system.h
index df781ad..d141e48 100644
--- a/include/asm-powerpc/system.h
+++ b/include/asm-powerpc/system.h
@@ -519,54 +519,6 @@ extern void reloc_got2(unsigned long);
 
 #define PTRRELOC(x)	((typeof(x)) add_reloc_offset((unsigned long)(x)))
 
-static inline void create_instruction(unsigned long addr, unsigned int instr)
-{
-	unsigned int *p;
-	p  = (unsigned int *)addr;
-	*p = instr;
-	asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (p));
-}
-
-/* Flags for create_branch:
- * "b"   == create_branch(addr, target, 0);
- * "ba"  == create_branch(addr, target, BRANCH_ABSOLUTE);
- * "bl"  == create_branch(addr, target, BRANCH_SET_LINK);
- * "bla" == create_branch(addr, target, BRANCH_ABSOLUTE | BRANCH_SET_LINK);
- */
-#define BRANCH_SET_LINK	0x1
-#define BRANCH_ABSOLUTE	0x2
-
-static inline void create_branch(unsigned long addr,
-		unsigned long target, int flags)
-{
-	unsigned int instruction;
-
-	if (! (flags & BRANCH_ABSOLUTE))
-		target = target - addr;
-
-	/* Mask out the flags and target, so they don't step on each other. */
-	instruction = 0x48000000 | (flags & 0x3) | (target & 0x03FFFFFC);
-
-	create_instruction(addr, instruction);
-}
-
-static inline void create_function_call(unsigned long addr, void * func)
-{
-	unsigned long func_addr;
-
-#ifdef CONFIG_PPC64
-	/*
-	 * On PPC64 the function pointer actually points to the function's
-	 * descriptor. The first entry in the descriptor is the address
-	 * of the function text.
-	 */
-	func_addr = *(unsigned long *)func;
-#else
-	func_addr = (unsigned long)func;
-#endif
-	create_branch(addr, func_addr, BRANCH_SET_LINK);
-}
-
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING
 extern void account_system_vtime(struct task_struct *);
 #endif
-- 
1.5.5

[PATCH 2/6] Allow create_branch() to return errors

[Michael Ellerman]

Currently create_branch() creates a branch instruction for you, and patches
it into the call site. In some circumstances it would be nice to be able to
create the instruction and patch it later, and also some code might want
to check for errors in the branch creation before doing the patching. A
future patch will change create_branch() to check for errors.

For callers that don't care, replace create_branch() with patch_branch(),
which just creates the branch and patches it directly.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/crash_dump.c          |    4 ++--
 arch/powerpc/lib/code-patching.c          |   11 ++++++++---
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |    2 +-
 arch/powerpc/platforms/powermac/smp.c     |    2 +-
 include/asm-powerpc/code-patching.h       |    5 +++--
 5 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index 35b9a66..ee2dac5 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -42,8 +42,8 @@ static void __init create_trampoline(unsigned long addr)
 	 * branch to "addr" we jump to ("addr" + 32 MB). Although it requires
 	 * two instructions it doesn't require any registers.
 	 */
-	create_instruction(addr, 0x60000000); /* nop */
-	create_branch(addr + 4, addr + PHYSICAL_START, 0);
+	patch_instruction(addr, 0x60000000); /* nop */
+	patch_branch(addr + 4, addr + PHYSICAL_START, 0);
 }
 
 void __init setup_kdump_trampoline(void)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 7afae88..1391981 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -11,7 +11,7 @@
 #include <asm/code-patching.h>
 
 
-void create_instruction(unsigned long addr, unsigned int instr)
+void patch_instruction(unsigned long addr, unsigned int instr)
 {
 	unsigned int *p;
 	p  = (unsigned int *)addr;
@@ -19,7 +19,12 @@ void create_instruction(unsigned long addr, unsigned int instr)
 	asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (p));
 }
 
-void create_branch(unsigned long addr, unsigned long target, int flags)
+void patch_branch(unsigned long addr, unsigned long target, int flags)
+{
+	patch_instruction(addr, create_branch(addr, target, flags));
+}
+
+unsigned int create_branch(unsigned long addr, unsigned long target, int flags)
 {
 	unsigned int instruction;
 
@@ -29,5 +34,5 @@ void create_branch(unsigned long addr, unsigned long target, int flags)
 	/* Mask out the flags and target, so they don't step on each other. */
 	instruction = 0x48000000 | (flags & 0x3) | (target & 0x03FFFFFC);
 
-	create_instruction(addr, instruction);
+	return instruction;
 }
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
index 63f5585..7c5ac68 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
@@ -72,7 +72,7 @@ smp_86xx_kick_cpu(int nr)
 
 	/* Setup fake reset vector to call __secondary_start_mpc86xx. */
 	target = (unsigned long) __secondary_start_mpc86xx;
-	create_branch((unsigned long)vector, target, BRANCH_SET_LINK);
+	patch_branch((unsigned long)vector, target, BRANCH_SET_LINK);
 
 	/* Kick that CPU */
 	smp_86xx_release_core(nr);
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index bf202f7..ff483cb 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -805,7 +805,7 @@ static void __devinit smp_core99_kick_cpu(int nr)
 	 *   b __secondary_start_pmac_0 + nr*8 - KERNELBASE
 	 */
 	target = (unsigned long) __secondary_start_pmac_0 + nr * 8;
-	create_branch((unsigned long)vector, target, BRANCH_SET_LINK);
+	patch_branch((unsigned long)vector, target, BRANCH_SET_LINK);
 
 	/* Put some life in our friend */
 	pmac_call_feature(PMAC_FTR_RESET_CPU, NULL, nr, 0);
diff --git a/include/asm-powerpc/code-patching.h b/include/asm-powerpc/code-patching.h
index 0b91fdf..41ecae8 100644
--- a/include/asm-powerpc/code-patching.h
+++ b/include/asm-powerpc/code-patching.h
@@ -19,7 +19,8 @@
 #define BRANCH_SET_LINK	0x1
 #define BRANCH_ABSOLUTE	0x2
 
-extern void create_branch(unsigned long addr, unsigned long target, int flags);
-extern void create_instruction(unsigned long addr, unsigned int instr);
+unsigned int create_branch(unsigned long addr, unsigned long target, int flags);
+void patch_branch(unsigned long addr, unsigned long target, int flags);
+void patch_instruction(unsigned long addr, unsigned int instr);
 
 #endif /* _ASM_POWERPC_CODE_PATCHING_H */
-- 
1.5.5

[PATCH 3/6] Make create_branch() return errors if the branch target is too large

[Michael Ellerman]

If you pass a target value to create_branch() which is more than 32MB - 4,
or - 32MB away from the branch site, then it's impossible to create an
immediate branch. The current code doesn't check, which will lead to us
creating a branch to somewhere else - which is bad.

For code that cares to check we return 0, which is easy to check for, and
for code that doesn't at least we'll be creating an illegal instruction,
rather than a branch to some random address.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/lib/code-patching.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 1391981..2905c51 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -27,12 +27,18 @@ void patch_branch(unsigned long addr, unsigned long target, int flags)
 unsigned int create_branch(unsigned long addr, unsigned long target, int flags)
 {
 	unsigned int instruction;
+	long offset;
 
+	offset = target;
 	if (! (flags & BRANCH_ABSOLUTE))
-		target = target - addr;
+		offset = offset - addr;
+
+	/* Check we can represent the target in the instruction format */
+	if (offset < -0x2000000 || offset > 0x1fffffc || offset & 0x3)
+		return 0;
 
 	/* Mask out the flags and target, so they don't step on each other. */
-	instruction = 0x48000000 | (flags & 0x3) | (target & 0x03FFFFFC);
+	instruction = 0x48000000 | (flags & 0x3) | (offset & 0x03FFFFFC);
 
 	return instruction;
 }
-- 
1.5.5

[PATCH 4/6] Add ppc_function_entry() which gets the entry point for a function

[Michael Ellerman]

Because function pointers point to different things on 32-bit vs 64-bit,
add a macro that deals with dereferencing the OPD on 64-bit. The soon to
be merged ftrace wants this, as well as other code I am working on.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 include/asm-powerpc/code-patching.h |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/asm-powerpc/code-patching.h b/include/asm-powerpc/code-patching.h
index 41ecae8..0213a48 100644
--- a/include/asm-powerpc/code-patching.h
+++ b/include/asm-powerpc/code-patching.h
@@ -10,6 +10,8 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <asm/types.h>
+
 /* Flags for create_branch:
  * "b"   == create_branch(addr, target, 0);
  * "ba"  == create_branch(addr, target, BRANCH_ABSOLUTE);
@@ -23,4 +25,18 @@ unsigned int create_branch(unsigned long addr, unsigned long target, int flags);
 void patch_branch(unsigned long addr, unsigned long target, int flags);
 void patch_instruction(unsigned long addr, unsigned int instr);
 
+static inline unsigned long ppc_function_entry(void *func)
+{
+#ifdef CONFIG_PPC64
+	/*
+	 * On PPC64 the function pointer actually points to the function's
+	 * descriptor. The first entry in the descriptor is the address
+	 * of the function text.
+	 */
+	return ((func_descr_t *)func)->entry;
+#else
+	return (unsigned long)func;
+#endif
+}
+
 #endif /* _ASM_POWERPC_CODE_PATCHING_H */
-- 
1.5.5

[PATCH 5/6] Add tests for the code patching code, and introduce checking functions

[Michael Ellerman]

Some code I am working on wants to check that the code it's patching is
what it expects, so add three functions which check the passed instruction
to see what it is.

Add some tests for those routines, and also test the results of the existing
create_branch() function using the new routines.

It would be nice to have the test code enabled automatically for debug
kernels, but distros turn on DEBUG_KERNEL so we don't really have a config
symbol we can tie it off.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/Kconfig.debug          |    5 +
 arch/powerpc/lib/code-patching.c    |  140 +++++++++++++++++++++++++++++++++++
 include/asm-powerpc/code-patching.h |    4 +
 3 files changed, 149 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index a7d24e6..dc58939 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -57,6 +57,11 @@ config KGDB
 	  debugger.  See <http://kgdb.sourceforge.net/> for more information.
 	  Unless you are intending to debug the kernel, say N here.
 
+config CODE_PATCHING_SELFTEST
+	bool "Run self-tests of the code-patching code."
+	depends on DEBUG_KERNEL
+	default n
+
 choice
 	prompt "Serial Port"
 	depends on KGDB
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 2905c51..13e29eb 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/init.h>
 #include <asm/code-patching.h>
 
 
@@ -42,3 +43,142 @@ unsigned int create_branch(unsigned long addr, unsigned long target, int flags)
 
 	return instruction;
 }
+
+int instr_is_branch_iform(unsigned int instr)
+{
+	unsigned int opcode;
+
+	opcode = (instr >> 26) & 0x3F;
+
+	return opcode == 18;
+}
+
+int instr_is_bl(unsigned int instr)
+{
+	return instr_is_branch_iform(instr) &&
+	       ((instr & BRANCH_SET_LINK) == 1) &&
+	       ((instr & BRANCH_ABSOLUTE) == 0);
+}
+
+int instr_is_branch_to_addr(unsigned int *instr, unsigned long addr)
+{
+	signed long imm;
+
+	if (!instr_is_branch_iform(*instr))
+		return 0;
+
+	imm = *instr & 0x3FFFFFC;
+
+	/* If the top bit of the immediate value is set this is negative */
+	if (imm & 0x2000000)
+		imm -= 0x4000000;
+
+	if ((*instr & BRANCH_ABSOLUTE) == 0)
+		imm += (unsigned long)instr;
+
+	return (unsigned long)imm == addr;
+}
+
+#ifdef CONFIG_CODE_PATCHING_SELFTEST
+
+static void __init test_trampoline(void)
+{
+	asm ("nop;\n");
+}
+
+#define check(x)	\
+	if (!(x)) printk("code-patching: test failed at line %d\n", __LINE__);
+
+static int __init test_code_patching(void)
+{
+	unsigned long addr, dest;
+	unsigned int instr;
+
+	addr = (unsigned long)&instr;
+
+	printk(KERN_DEBUG "Running code patching self-tests ...\n");
+
+	/* The simplest case, branch to self, no flags */
+	check(instr_is_branch_iform(0x48000000));
+	/* All bits of target set, and flags */
+	check(instr_is_branch_iform(0x4bffffff));
+	/* High bit of opcode set, which is wrong */
+	check(!instr_is_branch_iform(0xcbffffff));
+	/* Middle bits of opcode set, which is wrong */
+	check(!instr_is_branch_iform(0x7bffffff));
+
+	/* Simplest case, branch to self, with link */
+	check(instr_is_bl(0x48000001));
+	/* All bits of targets set, and link */
+	check(instr_is_bl(0x4bfffffd));
+	/* Some bits of targets set, and link */
+	check(instr_is_bl(0x4bff00fd));
+	/* All bits of targets set, no flags, ie. link is false */
+	check(!instr_is_bl(0x4bfffffc));
+	/* Must be a valid branch to start with */
+	check(!instr_is_bl(0x7bfffffd));
+
+	/* Absolute branch to 0x100 */
+	instr = 0x48000103;
+	check(instr_is_branch_to_addr(&instr, 0x100));
+	/* Absolute branch to 0x420fc */
+	instr = 0x480420ff;
+	check(instr_is_branch_to_addr(&instr, 0x420fc));
+
+	/* Maximum positive relative branch, + 20MB - 4B */
+	instr = 0x49fffffc;
+	check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC));
+
+	/* Smallest negative relative branch, - 4B */
+	instr = 0x4bfffffc;
+	check(instr_is_branch_to_addr(&instr, addr - 4));
+
+	/* Largest negative relative branch, - 32 MB */
+	instr = 0x4a000000;
+	check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
+
+
+	/* Branch to self, with link */
+	instr = create_branch(addr, addr, BRANCH_SET_LINK);
+	check(instr_is_branch_to_addr(&instr, addr));
+	check(instr_is_bl(instr));
+
+	/* Branch to self - 0x100, with link */
+	instr = create_branch(addr, addr - 0x100, BRANCH_SET_LINK);
+	check(instr_is_branch_to_addr(&instr, addr - 0x100));
+	check(instr_is_bl(instr));
+
+	/* Branch to self + 0x100, no link */
+	instr = create_branch(addr, addr + 0x100, 0);
+	check(instr_is_branch_to_addr(&instr, addr + 0x100));
+	check(!instr_is_bl(instr));
+
+	/* Maximum relative negative offset, - 32 MB */
+	instr = create_branch(addr, addr - 0x2000000, BRANCH_SET_LINK);
+	check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
+	check(instr_is_bl(instr));
+
+	/* Out of range relative negative offset, - 32 MB + 4*/
+	instr = create_branch(addr, addr - 0x2000004, BRANCH_SET_LINK);
+	check(instr == 0);
+
+	/* Out of range relative positive offset, + 32 MB */
+	instr = create_branch(addr, addr + 0x2000000, BRANCH_SET_LINK);
+	check(instr == 0);
+
+	/* Unaligned target */
+	instr = create_branch(addr, addr + 3, BRANCH_SET_LINK);
+	check(instr == 0);
+
+	/* Check we can create a function call */
+	addr = ppc_function_entry(test_trampoline);
+	dest = ppc_function_entry(test_code_patching);
+	instr = create_branch(addr, dest, BRANCH_SET_LINK);
+	patch_instruction(addr, instr);
+	check(instr_is_branch_to_addr((unsigned int *)addr, dest));
+
+	return 0;
+}
+late_initcall(test_code_patching);
+
+#endif /* CONFIG_CODE_PATCHING_SELFTEST */
diff --git a/include/asm-powerpc/code-patching.h b/include/asm-powerpc/code-patching.h
index 0213a48..8b54373 100644
--- a/include/asm-powerpc/code-patching.h
+++ b/include/asm-powerpc/code-patching.h
@@ -25,6 +25,10 @@ unsigned int create_branch(unsigned long addr, unsigned long target, int flags);
 void patch_branch(unsigned long addr, unsigned long target, int flags);
 void patch_instruction(unsigned long addr, unsigned int instr);
 
+int instr_is_branch_iform(unsigned int instr);
+int instr_is_bl(unsigned int instr);
+int instr_is_branch_to_addr(unsigned int *instr, unsigned long addr);
+
 static inline unsigned long ppc_function_entry(void *func)
 {
 #ifdef CONFIG_PPC64
-- 
1.5.5

[PATCH 6/6] Add PPC_NOP_INSTR, a hash define for the preferred nop instruction

[Michael Ellerman]

A bunch of code has hard-coded the value for a "nop" instruction, it
would be nice to have a #define for it.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/kernel/cputable.c      |    3 ++-
 arch/powerpc/kernel/crash_dump.c    |    2 +-
 arch/powerpc/kernel/module_64.c     |    3 ++-
 include/asm-powerpc/code-patching.h |    2 ++
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index e44d553..887e190 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 
 #include <asm/oprofile_impl.h>
+#include <asm/code-patching.h>
 #include <asm/cputable.h>
 #include <asm/prom.h>		/* for PTRRELOC on ARCH=ppc */
 
@@ -1613,7 +1614,7 @@ void do_feature_fixups(unsigned long value, void *fixup_start, void *fixup_end)
 		pend = ((unsigned int *)fcur) + (fcur->end_off / 4);
 
 		for (p = pstart; p < pend; p++) {
-			*p = 0x60000000u;
+			*p = PPC_NOP_INSTR;
 			asm volatile ("dcbst 0, %0" : : "r" (p));
 		}
 		asm volatile ("sync" : : : "memory");
diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index ee2dac5..2f5e04f 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -42,7 +42,7 @@ static void __init create_trampoline(unsigned long addr)
 	 * branch to "addr" we jump to ("addr" + 32 MB). Although it requires
 	 * two instructions it doesn't require any registers.
 	 */
-	patch_instruction(addr, 0x60000000); /* nop */
+	patch_instruction(addr, PPC_NOP_INSTR); /* nop */
 	patch_branch(addr + 4, addr + PHYSICAL_START, 0);
 }
 
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 3a82b02..d5e569a 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -24,6 +24,7 @@
 #include <asm/module.h>
 #include <asm/uaccess.h>
 #include <asm/firmware.h>
+#include <asm/code-patching.h>
 #include <linux/sort.h>
 
 #include "setup.h"
@@ -346,7 +347,7 @@ static unsigned long stub_for_addr(Elf64_Shdr *sechdrs,
    restore r2. */
 static int restore_r2(u32 *instruction, struct module *me)
 {
-	if (*instruction != 0x60000000) {
+	if (*instruction != PPC_NOP_INSTR) {
 		printk("%s: Expect noop after relocate, got %08x\n",
 		       me->name, *instruction);
 		return 0;
diff --git a/include/asm-powerpc/code-patching.h b/include/asm-powerpc/code-patching.h
index 8b54373..f33d16e 100644
--- a/include/asm-powerpc/code-patching.h
+++ b/include/asm-powerpc/code-patching.h
@@ -12,6 +12,8 @@
 
 #include <asm/types.h>
 
+#define PPC_NOP_INSTR	0x60000000
+
 /* Flags for create_branch:
  * "b"   == create_branch(addr, target, 0);
  * "ba"  == create_branch(addr, target, BRANCH_ABSOLUTE);
-- 
1.5.5

Re: [PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[Benjamin Herrenschmidt]

On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> We currently have a few routines for patching code in asm/system.h, because
> they didn't fit anywhere else. I'd like to clean them up a little and add
> some more, so first move them into a dedicated C file - they don't need to
> be inlined.
> 
> While we're moving the code, drop create_function_call(), it's intended
> caller never got merged and will be replaced in future with something
> different.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH 2/6] Allow create_branch() to return errors

[Benjamin Herrenschmidt]

On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> Currently create_branch() creates a branch instruction for you, and patches
> it into the call site. In some circumstances it would be nice to be able to
> create the instruction and patch it later, and also some code might want
> to check for errors in the branch creation before doing the patching. A
> future patch will change create_branch() to check for errors.
> 
> For callers that don't care, replace create_branch() with patch_branch(),
> which just creates the branch and patches it directly.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH 3/6] Make create_branch() return errors if the branch target is too large

[Benjamin Herrenschmidt]

On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> If you pass a target value to create_branch() which is more than 32MB - 4,
> or - 32MB away from the branch site, then it's impossible to create an
> immediate branch. The current code doesn't check, which will lead to us
> creating a branch to somewhere else - which is bad.
> 
> For code that cares to check we return 0, which is easy to check for, and
> for code that doesn't at least we'll be creating an illegal instruction,
> rather than a branch to some random address.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH 4/6] Add ppc_function_entry() which gets the entry point for a function

[Benjamin Herrenschmidt]

On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> Because function pointers point to different things on 32-bit vs
> 64-bit,
> add a macro that deals with dereferencing the OPD on 64-bit. The soon
> to
> be merged ftrace wants this, as well as other code I am working on.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Should we be smarter and also compare if the TOC pointer is the same
between the source and target and if not, warn loudly ?

Or we just don't care ?

Ben.

Re: [PATCH 4/6] Add ppc_function_entry() which gets the entry point for a function

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Thu, 2008-05-29 at 16:37 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> > Because function pointers point to different things on 32-bit vs
> > 64-bit,
> > add a macro that deals with dereferencing the OPD on 64-bit. The soon
> > to
> > be merged ftrace wants this, as well as other code I am working on.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Should we be smarter and also compare if the TOC pointer is the same
> between the source and target and if not, warn loudly ?

> Or we just don't care ?

I think currently only the module loader worries about changing TOC
pointers, but perhaps we should think about it in other places too. What
do you do on 32-bit though?

Still, I think that would be a new separate macro, so this stands on its
own IMHO.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/6] Allow create_branch() to return errors

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2539 bytes --]

On Thu, 2008-05-29 at 16:20 +1000, Michael Ellerman wrote:
> Currently create_branch() creates a branch instruction for you, and patches
> it into the call site. In some circumstances it would be nice to be able to
> create the instruction and patch it later, and also some code might want
> to check for errors in the branch creation before doing the patching. A
> future patch will change create_branch() to check for errors.
> 
> For callers that don't care, replace create_branch() with patch_branch(),
> which just creates the branch and patches it directly.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
>  arch/powerpc/kernel/crash_dump.c          |    4 ++--
>  arch/powerpc/lib/code-patching.c          |   11 ++++++++---
>  arch/powerpc/platforms/86xx/mpc86xx_smp.c |    2 +-
>  arch/powerpc/platforms/powermac/smp.c     |    2 +-
>  include/asm-powerpc/code-patching.h       |    5 +++--
>  5 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
> index 35b9a66..ee2dac5 100644
> --- a/arch/powerpc/kernel/crash_dump.c
> +++ b/arch/powerpc/kernel/crash_dump.c
> @@ -42,8 +42,8 @@ static void __init create_trampoline(unsigned long addr)
>  	 * branch to "addr" we jump to ("addr" + 32 MB). Although it requires
>  	 * two instructions it doesn't require any registers.
>  	 */
> -	create_instruction(addr, 0x60000000); /* nop */
> -	create_branch(addr + 4, addr + PHYSICAL_START, 0);
> +	patch_instruction(addr, 0x60000000); /* nop */
> +	patch_branch(addr + 4, addr + PHYSICAL_START, 0);
>  }
>  
>  void __init setup_kdump_trampoline(void)
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index 7afae88..1391981 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -11,7 +11,7 @@
>  #include <asm/code-patching.h>
>  
> 
> -void create_instruction(unsigned long addr, unsigned int instr)
> +void patch_instruction(unsigned long addr, unsigned int instr)
>  {
>  	unsigned int *p;
>  	p  = (unsigned int *)addr;

Reviewing my own patches again .. this should take an unsigned int *
instead of an unsigned long.

I'll repost sometime.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[Kumar Gala]

On May 29, 2008, at 1:20 AM, Michael Ellerman wrote:

> We currently have a few routines for patching code in asm/system.h,  
> because
> they didn't fit anywhere else. I'd like to clean them up a little  
> and add
> some more, so first move them into a dedicated C file - they don't  
> need to
> be inlined.
>
> While we're moving the code, drop create_function_call(), it's  
> intended
> caller never got merged and will be replaced in future with something
> different.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/kernel/crash_dump.c          |    1 +
> arch/powerpc/lib/Makefile                 |    2 +
> arch/powerpc/lib/code-patching.c          |   33 ++++++++++++++++++++
> arch/powerpc/platforms/86xx/mpc86xx_smp.c |    1 +
> arch/powerpc/platforms/powermac/smp.c     |    1 +
> include/asm-powerpc/code-patching.h       |   25 +++++++++++++++
> include/asm-powerpc/system.h              |   48  
> -----------------------------
> 7 files changed, 63 insertions(+), 48 deletions(-)
>
> diff --git a/arch/powerpc/kernel/crash_dum

what's the state of these patches and getting them into powerpc-next?

I'm looking at some runtime fix ups that I was thinking of basing on  
this code.

- k

Re: [PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

On Thu, 2008-06-19 at 01:15 -0500, Kumar Gala wrote:
> On May 29, 2008, at 1:20 AM, Michael Ellerman wrote:
> 
> > We currently have a few routines for patching code in asm/system.h,  
> > because
> > they didn't fit anywhere else. I'd like to clean them up a little  
> > and add
> > some more, so first move them into a dedicated C file - they don't  
> > need to
> > be inlined.
> >
> > While we're moving the code, drop create_function_call(), it's  
> > intended
> > caller never got merged and will be replaced in future with something
> > different.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> > arch/powerpc/kernel/crash_dump.c          |    1 +
> > arch/powerpc/lib/Makefile                 |    2 +
> > arch/powerpc/lib/code-patching.c          |   33 ++++++++++++++++++++
> > arch/powerpc/platforms/86xx/mpc86xx_smp.c |    1 +
> > arch/powerpc/platforms/powermac/smp.c     |    1 +
> > include/asm-powerpc/code-patching.h       |   25 +++++++++++++++
> > include/asm-powerpc/system.h              |   48  
> > -----------------------------
> > 7 files changed, 63 insertions(+), 48 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/crash_dum
> 
> what's the state of these patches and getting them into powerpc-next?

I think what I posted is reasonably solid, I've added some more routines
for the stuff I'm working on. I'll repost today or tommorrow.

> I'm looking at some runtime fix ups that I was thinking of basing on  
> this code.

What have you got in mind? I'm working on some runtime fixups too :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/6] Move code patching code into arch/powerpc/lib/code-patching.c

[Kumar Gala]

On Jun 19, 2008, at 1:55 AM, Michael Ellerman wrote:

> On Thu, 2008-06-19 at 01:15 -0500, Kumar Gala wrote:
>> On May 29, 2008, at 1:20 AM, Michael Ellerman wrote:
>>
>>> We currently have a few routines for patching code in asm/system.h,
>>> because
>>> they didn't fit anywhere else. I'd like to clean them up a little
>>> and add
>>> some more, so first move them into a dedicated C file - they don't
>>> need to
>>> be inlined.
>>>
>>> While we're moving the code, drop create_function_call(), it's
>>> intended
>>> caller never got merged and will be replaced in future with  
>>> something
>>> different.
>>>
>>> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
>>> ---
>>> arch/powerpc/kernel/crash_dump.c          |    1 +
>>> arch/powerpc/lib/Makefile                 |    2 +
>>> arch/powerpc/lib/code-patching.c          |   33 ++++++++++++++++++ 
>>> ++
>>> arch/powerpc/platforms/86xx/mpc86xx_smp.c |    1 +
>>> arch/powerpc/platforms/powermac/smp.c     |    1 +
>>> include/asm-powerpc/code-patching.h       |   25 +++++++++++++++
>>> include/asm-powerpc/system.h              |   48
>>> -----------------------------
>>> 7 files changed, 63 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/crash_dum
>>
>> what's the state of these patches and getting them into powerpc-next?
>
> I think what I posted is reasonably solid, I've added some more  
> routines
> for the stuff I'm working on. I'll repost today or tommorrow.
>
>> I'm looking at some runtime fix ups that I was thinking of basing on
>> this code.
>
> What have you got in mind? I'm working on some runtime fixups too :)

I want to be able to run time fix up lwsync an remove it as compile  
time thing.

- k