[PATCH 2/2]: sparc64: Add ftrace support.

[PATCH 2/2]: sparc64: Add ftrace support.

[David Miller]

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc64/Kconfig         |    1 +
 arch/sparc64/Kconfig.debug   |    2 +-
 arch/sparc64/kernel/Makefile |    1 +
 arch/sparc64/kernel/ftrace.c |   99 ++++++++++++++++++++++++++++++++++++++++++
 arch/sparc64/lib/mcount.S    |   58 +++++++++++++++++++++++--
 5 files changed, 156 insertions(+), 5 deletions(-)
 create mode 100644 arch/sparc64/kernel/ftrace.c

diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 4c40862..56344b1 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -15,6 +15,7 @@ config SPARC64
 	select HAVE_LMB
 	select HAVE_ARCH_KGDB
 	select HAVE_IMMEDIATE
+	select HAVE_FTRACE
 
 config GENERIC_TIME
 	bool
diff --git a/arch/sparc64/Kconfig.debug b/arch/sparc64/Kconfig.debug
index 6a4d28a..d6d32d1 100644
--- a/arch/sparc64/Kconfig.debug
+++ b/arch/sparc64/Kconfig.debug
@@ -33,7 +33,7 @@ config DEBUG_PAGEALLOC
 
 config MCOUNT
 	bool
-	depends on STACK_DEBUG
+	depends on STACK_DEBUG || FTRACE
 	default y
 
 config FRAME_POINTER
diff --git a/arch/sparc64/kernel/Makefile b/arch/sparc64/kernel/Makefile
index 311c797..52e933f 100644
--- a/arch/sparc64/kernel/Makefile
+++ b/arch/sparc64/kernel/Makefile
@@ -14,6 +14,7 @@ obj-y		:= process.o setup.o cpu.o idprom.o \
 		   power.o sbus.o sparc64_ksyms.o chmc.o \
 		   visemul.o prom.o of_device.o hvapi.o sstate.o mdesc.o
 
+obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
 obj-$(CONFIG_STACKTRACE) += stacktrace.o
 obj-$(CONFIG_PCI)	 += ebus.o pci_common.o \
 			    pci_psycho.o pci_sabre.o pci_schizo.o \
diff --git a/arch/sparc64/kernel/ftrace.c b/arch/sparc64/kernel/ftrace.c
new file mode 100644
index 0000000..f449e6d
--- /dev/null
+++ b/arch/sparc64/kernel/ftrace.c
@@ -0,0 +1,99 @@
+#include <linux/spinlock.h>
+#include <linux/hardirq.h>
+#include <linux/ftrace.h>
+#include <linux/percpu.h>
+#include <linux/init.h>
+#include <linux/list.h>
+
+static const u32 ftrace_nop = 0x01000000;
+
+notrace int ftrace_ip_converted(unsigned long ip)
+{
+	u32 insn = *(u32 *) ip;
+
+	return (insn == ftrace_nop);
+}
+
+notrace unsigned char *ftrace_nop_replace(void)
+{
+	return (char *)&ftrace_nop;
+}
+
+notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
+{
+	static u32 call;
+	s32 off;
+
+	off = ((s32)addr - (s32)ip);
+	call = 0x40000000 | ((u32)off >> 2);
+
+	return (unsigned char *) &call;
+}
+
+notrace int
+ftrace_modify_code(unsigned long ip, unsigned char *old_code,
+		   unsigned char *new_code)
+{
+	u32 old = *(u32 *)old_code;
+	u32 new = *(u32 *)new_code;
+	u32 replaced;
+	int faulted;
+
+	__asm__ __volatile__(
+	"1:	cas	[%[ip]], %[old], %[new]\n"
+	"	flush	%[ip]\n"
+	"	mov	0, %[faulted]\n"
+	"2:\n"
+	"	.section .fixup,#alloc,#execinstr\n"
+	"	.align	4\n"
+	"3:	sethi	%%hi(2b), %[faulted]\n"
+	"	jmpl	%[faulted] + %%lo(2b), %%g0\n"
+	"	 mov	1, %[faulted]\n"
+	"	.previous\n"
+	"	.section __ex_table,\"a\"\n"
+	"	.align	4\n"
+	"	.word	1b, 3b\n"
+	"	.previous\n"
+	: "=r" (replaced), [faulted] "=r" (faulted)
+	: [new] "0" (new), [old] "r" (old), [ip] "r" (ip)
+	: "memory");
+
+	if (replaced != old && replaced != new)
+		faulted = 2;
+
+	return faulted;
+}
+
+notrace int ftrace_update_ftrace_func(ftrace_func_t func)
+{
+	unsigned long ip = (unsigned long)(&ftrace_call);
+	unsigned char old[4], *new;
+
+	memcpy(old, &ftrace_call, 4);
+	new = ftrace_call_replace(ip, (unsigned long)func);
+	return ftrace_modify_code(ip, old, new);
+}
+
+notrace int ftrace_mcount_set(unsigned long *data)
+{
+	unsigned long ip = (long)(&mcount_call);
+	unsigned long *addr = data;
+	unsigned char old[4], *new;
+
+	/*
+	 * Replace the mcount stub with a pointer to the
+	 * ip recorder function.
+	 */
+	memcpy(old, &mcount_call, 4);
+	new = ftrace_call_replace(ip, *addr);
+	*addr = ftrace_modify_code(ip, old, new);
+
+	return 0;
+}
+
+
+int __init ftrace_dyn_arch_init(void *data)
+{
+	ftrace_mcount_set(data);
+	return 0;
+}
diff --git a/arch/sparc64/lib/mcount.S b/arch/sparc64/lib/mcount.S
index 9e4534b..7735a7a 100644
--- a/arch/sparc64/lib/mcount.S
+++ b/arch/sparc64/lib/mcount.S
@@ -28,10 +28,13 @@ ovstack:
 	.skip		OVSTACKSIZE
 #endif
 	.text
-	.align 32
-	.globl mcount, _mcount
-mcount:
+	.align		32
+	.globl		_mcount
+	.type		_mcount,#function
+	.globl		mcount
+	.type		mcount,#function
 _mcount:
+mcount:
 #ifdef CONFIG_STACK_DEBUG
 	/*
 	 * Check whether %sp is dangerously low.
@@ -55,6 +58,53 @@ _mcount:
 	 or		%g3, %lo(panicstring), %o0
 	call		prom_halt
 	 nop
+1:
+#endif
+#ifdef CONFIG_FTRACE
+#ifdef CONFIG_DYNAMIC_FTRACE
+	mov		%o7, %o0
+	.globl		mcount_call
+mcount_call:
+	call		ftrace_stub
+	 mov		%o0, %o7
+#else
+	sethi		%hi(ftrace_trace_function), %g1
+	sethi		%hi(ftrace_stub), %g2
+	ldx		[%g1 + %lo(ftrace_trace_function)], %g1
+	or		%g2, %lo(ftrace_stub), %g2
+	cmp		%g1, %g2
+	be,pn		%icc, 1f
+	 mov		%i7, %o1
+	jmpl		%g1, %g0
+	 mov		%o7, %o0
+	/* not reached */
+1:
 #endif
-1:	retl
+#endif
+	retl
 	 nop
+	.size		_mcount,.-_mcount
+	.size		mcount,.-mcount
+
+#ifdef CONFIG_FTRACE
+	.globl		ftrace_stub
+	.type		ftrace_stub,#function
+ftrace_stub:
+	retl
+	 nop
+	.size		ftrace_stub,.-ftrace_stub
+#ifdef CONFIG_DYNAMIC_FTRACE
+	.globl		ftrace_caller
+	.type		ftrace_caller,#function
+ftrace_caller:
+	mov		%i7, %o1
+	mov		%o7, %o0
+	.globl		ftrace_call
+ftrace_call:
+	call		ftrace_stub
+	 mov		%o0, %o7
+	retl
+	 nop
+	.size		ftrace_caller,.-ftrace_caller
+#endif
+#endif
-- 
1.5.5.rc0.16.g02b00

Re: [PATCH 2/2]: sparc64: Add ftrace support.

[Steven Rostedt]

David Miller wrote:
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  arch/sparc64/Kconfig         |    1 +
>  arch/sparc64/Kconfig.debug   |    2 +-
>  arch/sparc64/kernel/Makefile |    1 +
>  arch/sparc64/kernel/ftrace.c |   99 ++++++++++++++++++++++++++++++++++++++++++
>  arch/sparc64/lib/mcount.S    |   58 +++++++++++++++++++++++--
>  5 files changed, 156 insertions(+), 5 deletions(-)
>  create mode 100644 arch/sparc64/kernel/ftrace.c
> 
> diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
> index 4c40862..56344b1 100644
> --- a/arch/sparc64/Kconfig
> +++ b/arch/sparc64/Kconfig
> @@ -15,6 +15,7 @@ config SPARC64
>  	select HAVE_LMB
>  	select HAVE_ARCH_KGDB
>  	select HAVE_IMMEDIATE
> +	select HAVE_FTRACE
>  
>  config GENERIC_TIME
>  	bool
> diff --git a/arch/sparc64/Kconfig.debug b/arch/sparc64/Kconfig.debug
> index 6a4d28a..d6d32d1 100644
> --- a/arch/sparc64/Kconfig.debug
> +++ b/arch/sparc64/Kconfig.debug
> @@ -33,7 +33,7 @@ config DEBUG_PAGEALLOC
>  
>  config MCOUNT
>  	bool
> -	depends on STACK_DEBUG
> +	depends on STACK_DEBUG || FTRACE
>  	default y
>  
>  config FRAME_POINTER
> diff --git a/arch/sparc64/kernel/Makefile b/arch/sparc64/kernel/Makefile
> index 311c797..52e933f 100644
> --- a/arch/sparc64/kernel/Makefile
> +++ b/arch/sparc64/kernel/Makefile
> @@ -14,6 +14,7 @@ obj-y		:= process.o setup.o cpu.o idprom.o \
>  		   power.o sbus.o sparc64_ksyms.o chmc.o \
>  		   visemul.o prom.o of_device.o hvapi.o sstate.o mdesc.o
>  
> +obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
>  obj-$(CONFIG_STACKTRACE) += stacktrace.o
>  obj-$(CONFIG_PCI)	 += ebus.o pci_common.o \
>  			    pci_psycho.o pci_sabre.o pci_schizo.o \
> diff --git a/arch/sparc64/kernel/ftrace.c b/arch/sparc64/kernel/ftrace.c
> new file mode 100644
> index 0000000..f449e6d
> --- /dev/null
> +++ b/arch/sparc64/kernel/ftrace.c
> @@ -0,0 +1,99 @@
> +#include <linux/spinlock.h>
> +#include <linux/hardirq.h>
> +#include <linux/ftrace.h>
> +#include <linux/percpu.h>
> +#include <linux/init.h>
> +#include <linux/list.h>
> +
> +static const u32 ftrace_nop = 0x01000000;
> +
> +notrace int ftrace_ip_converted(unsigned long ip)

FYI,

If you look in the kernel/Makefile you will see a way to have the whole 
file not be traced. This will eliminate the need for the "notrace" 
annotation on all functions. I haven't update the x86 version of ftrace 
to do this so you probably just copied the old method from us.

If you add the following in arch/sparc64/kernel/Makefile

ifdef CONFIG_DYNAMIC_FTRACE
obj-y += ftrace.o
# Do not trace the ftrace code itself
ORIG_CFLAGS := $(KBUILD_CFLAGS)
KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
	$(ORIG_CFLAGS), \
	$(subst -pg,,$(ORIG_CFLAGS)))
endif

Then you wont need the "notrace" annotations on the functions.

/me needs to send Ingo a patch to do this for x86 as well.

> +{
> +	u32 insn = *(u32 *) ip;
> +
> +	return (insn == ftrace_nop);
> +}
> +
> +notrace unsigned char *ftrace_nop_replace(void)
> +{
> +	return (char *)&ftrace_nop;
> +}
> +
> +notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
> +{
> +	static u32 call;
> +	s32 off;
> +
> +	off = ((s32)addr - (s32)ip);
> +	call = 0x40000000 | ((u32)off >> 2);
> +
> +	return (unsigned char *) &call;
> +}
> +
> +notrace int
> +ftrace_modify_code(unsigned long ip, unsigned char *old_code,
> +		   unsigned char *new_code)
> +{
> +	u32 old = *(u32 *)old_code;
> +	u32 new = *(u32 *)new_code;
> +	u32 replaced;
> +	int faulted;
> +
> +	__asm__ __volatile__(
> +	"1:	cas	[%[ip]], %[old], %[new]\n"
> +	"	flush	%[ip]\n"
> +	"	mov	0, %[faulted]\n"
> +	"2:\n"
> +	"	.section .fixup,#alloc,#execinstr\n"
> +	"	.align	4\n"
> +	"3:	sethi	%%hi(2b), %[faulted]\n"
> +	"	jmpl	%[faulted] + %%lo(2b), %%g0\n"
> +	"	 mov	1, %[faulted]\n"
> +	"	.previous\n"
> +	"	.section __ex_table,\"a\"\n"
> +	"	.align	4\n"
> +	"	.word	1b, 3b\n"
> +	"	.previous\n"
> +	: "=r" (replaced), [faulted] "=r" (faulted)
> +	: [new] "0" (new), [old] "r" (old), [ip] "r" (ip)
> +	: "memory");

I'm so use to the %1 %2 and %3's I should update the x86 code to use the 
names as well. (nice)

> +
> +	if (replaced != old && replaced != new)
> +		faulted = 2;

Thanks for putting in the "2", I may plan on doing something different 
later if the code didn't "fault" but something else happened. I should 
also make MACROS out of that and not uses 1 and 2.

> +
> +	return faulted;
> +}
> +
> +notrace int ftrace_update_ftrace_func(ftrace_func_t func)
> +{
> +	unsigned long ip = (unsigned long)(&ftrace_call);
> +	unsigned char old[4], *new;
> +
> +	memcpy(old, &ftrace_call, 4);
> +	new = ftrace_call_replace(ip, (unsigned long)func);
> +	return ftrace_modify_code(ip, old, new);
> +}
> +
> +notrace int ftrace_mcount_set(unsigned long *data)
> +{
> +	unsigned long ip = (long)(&mcount_call);
> +	unsigned long *addr = data;
> +	unsigned char old[4], *new;
> +
> +	/*
> +	 * Replace the mcount stub with a pointer to the
> +	 * ip recorder function.
> +	 */
> +	memcpy(old, &mcount_call, 4);
> +	new = ftrace_call_replace(ip, *addr);
> +	*addr = ftrace_modify_code(ip, old, new);
> +
> +	return 0;
> +}
> +
> +
> +int __init ftrace_dyn_arch_init(void *data)
> +{
> +	ftrace_mcount_set(data);
> +	return 0;
> +}
> diff --git a/arch/sparc64/lib/mcount.S b/arch/sparc64/lib/mcount.S
> index 9e4534b..7735a7a 100644
> --- a/arch/sparc64/lib/mcount.S
> +++ b/arch/sparc64/lib/mcount.S
> @@ -28,10 +28,13 @@ ovstack:
>  	.skip		OVSTACKSIZE
>  #endif
>  	.text
> -	.align 32
> -	.globl mcount, _mcount
> -mcount:
> +	.align		32
> +	.globl		_mcount
> +	.type		_mcount,#function
> +	.globl		mcount
> +	.type		mcount,#function
>  _mcount:
> +mcount:
>  #ifdef CONFIG_STACK_DEBUG
>  	/*
>  	 * Check whether %sp is dangerously low.
> @@ -55,6 +58,53 @@ _mcount:
>  	 or		%g3, %lo(panicstring), %o0
>  	call		prom_halt
>  	 nop
> +1:
> +#endif
> +#ifdef CONFIG_FTRACE
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +	mov		%o7, %o0
> +	.globl		mcount_call
> +mcount_call:
> +	call		ftrace_stub
> +	 mov		%o0, %o7

WOW! Sparc64 has some really efficient context saving!

> +#else
> +	sethi		%hi(ftrace_trace_function), %g1
> +	sethi		%hi(ftrace_stub), %g2
> +	ldx		[%g1 + %lo(ftrace_trace_function)], %g1
> +	or		%g2, %lo(ftrace_stub), %g2
> +	cmp		%g1, %g2
> +	be,pn		%icc, 1f
> +	 mov		%i7, %o1
> +	jmpl		%g1, %g0
> +	 mov		%o7, %o0
> +	/* not reached */
> +1:
>  #endif
> -1:	retl
> +#endif
> +	retl
>  	 nop
> +	.size		_mcount,.-_mcount
> +	.size		mcount,.-mcount
> +
> +#ifdef CONFIG_FTRACE
> +	.globl		ftrace_stub
> +	.type		ftrace_stub,#function
> +ftrace_stub:
> +	retl
> +	 nop
> +	.size		ftrace_stub,.-ftrace_stub
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +	.globl		ftrace_caller
> +	.type		ftrace_caller,#function
> +ftrace_caller:
> +	mov		%i7, %o1
> +	mov		%o7, %o0
> +	.globl		ftrace_call
> +ftrace_call:
> +	call		ftrace_stub
> +	 mov		%o0, %o7
> +	retl
> +	 nop
> +	.size		ftrace_caller,.-ftrace_caller
> +#endif
> +#endif

Looks great!

Acked-by: Steven Rostedt <srostedt@redhat.com>

-- Steve

Re: [PATCH 2/2]: sparc64: Add ftrace support.

[David Miller]

From: Steven Rostedt <srostedt@redhat.com>
Date: Wed, 14 May 2008 09:32:18 -0400

> If you add the following in arch/sparc64/kernel/Makefile
> 
> ifdef CONFIG_DYNAMIC_FTRACE
> obj-y += ftrace.o
> # Do not trace the ftrace code itself
> ORIG_CFLAGS := $(KBUILD_CFLAGS)
> KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
> 	$(ORIG_CFLAGS), \
> 	$(subst -pg,,$(ORIG_CFLAGS)))
> endif
> 
> Then you wont need the "notrace" annotations on the functions.

That frankly looks like crap compared to the simple "notrace"
annotations in the *.c file.

This Makefile turd is in fact seemingly multiplying in various
places in the ftrace tree, please reverse this trend.

Re: [PATCH 2/2]: sparc64: Add ftrace support.

[Steven Rostedt]

David Miller wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> Date: Wed, 14 May 2008 09:32:18 -0400
> 
>> If you add the following in arch/sparc64/kernel/Makefile
>>
>> ifdef CONFIG_DYNAMIC_FTRACE
>> obj-y += ftrace.o
>> # Do not trace the ftrace code itself
>> ORIG_CFLAGS := $(KBUILD_CFLAGS)
>> KBUILD_CFLAGS = $(if $(filter-out ftrace, $(basename $(notdir $@))), \
>> 	$(ORIG_CFLAGS), \
>> 	$(subst -pg,,$(ORIG_CFLAGS)))
>> endif
>>
>> Then you wont need the "notrace" annotations on the functions.
> 
> That frankly looks like crap compared to the simple "notrace"
> annotations in the *.c file.
> 
> This Makefile turd is in fact seemingly multiplying in various
> places in the ftrace tree, please reverse this trend.

I wish it wasn't so ugly. The Makefile scripts allow you to add more 
CFLAGS to specific files but does not have a way to remove generic 
flags. If there was a better way then I would love to do it.

The problem is that adding "notrace" on ever function that needs it, 
also becomes quite messy. Especially when you have files where all 
functions in the file needs a notrace and there is 40 or so small 
functions that this applies to.

To add flags to file foo.c one simply needs to do

CFLAGS_foo.o = -newflags

It would be nice to have something like

CFLAGS_REMOVE_foo.o = -pg

Perhaps that is possible to do. Then the above "Makefile turd" wont be 
so crappy.

-- Steve

Re: [PATCH 2/2]: sparc64: Add ftrace support.

[David Miller]

From: Steven Rostedt <srostedt@redhat.com>
Date: Wed, 14 May 2008 17:58:13 -0400

> It would be nice to have something like
> 
> CFLAGS_REMOVE_foo.o = -pg
> 
> Perhaps that is possible to do. Then the above "Makefile turd" wont be 
> so crappy.

That would be much nicer.