diff mbox

[PATCHv3,5/5] arm64: Trace emulation of AArch32 legacy instructions

Message ID 1414435207-30240-7-git-send-email-punit.agrawal@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Punit Agrawal Oct. 27, 2014, 6:40 p.m. UTC
Introduce an event to trace the usage of emulated instructions. The
trace event is intended to help identify and encourage the migration
of legacy software using the emulation features.

Use this event to trace usage of swp and CP15 barrier emulation.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 arch/arm64/kernel/Makefile                 |  1 +
 arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
 arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/kernel/trace-events-emulation.h

Comments

Catalin Marinas Nov. 5, 2014, 2:46 p.m. UTC | #1
On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
> Introduce an event to trace the usage of emulated instructions. The
> trace event is intended to help identify and encourage the migration
> of legacy software using the emulation features.
> 
> Use this event to trace usage of swp and CP15 barrier emulation.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  arch/arm64/kernel/Makefile                 |  1 +
>  arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
>  arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/kernel/trace-events-emulation.h
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 5362578..1fc7abd 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -5,6 +5,7 @@
>  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> +CFLAGS_armv8_deprecated.o := -I$(src)

Why do you need this?

>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index fded15f..d376fe2 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -15,6 +15,9 @@
>  #include <linux/slab.h>
>  #include <linux/sysctl.h>
>  
> +#define CREATE_TRACE_POINTS
> +#include "trace-events-emulation.h"

Using double quotes should be fine for the current directory.
Steven Rostedt Nov. 5, 2014, 3:05 p.m. UTC | #2
On Wed, 5 Nov 2014 14:46:19 +0000
Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
> > Introduce an event to trace the usage of emulated instructions. The
> > trace event is intended to help identify and encourage the migration
> > of legacy software using the emulation features.
> > 
> > Use this event to trace usage of swp and CP15 barrier emulation.
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> > ---
> >  arch/arm64/kernel/Makefile                 |  1 +
> >  arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
> >  arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
> >  3 files changed, 58 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/arm64/kernel/trace-events-emulation.h
> > 
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index 5362578..1fc7abd 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -5,6 +5,7 @@
> >  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> >  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> >  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> > +CFLAGS_armv8_deprecated.o := -I$(src)
> 
> Why do you need this?

It has to do with the TRACE_EVENT magic.

Read samples/trace_events/Makefile

> 
> >  CFLAGS_REMOVE_ftrace.o = -pg
> >  CFLAGS_REMOVE_insn.o = -pg
> > diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> > index fded15f..d376fe2 100644
> > --- a/arch/arm64/kernel/armv8_deprecated.c
> > +++ b/arch/arm64/kernel/armv8_deprecated.c
> > @@ -15,6 +15,9 @@
> >  #include <linux/slab.h>
> >  #include <linux/sysctl.h>
> >  
> > +#define CREATE_TRACE_POINTS
> > +#include "trace-events-emulation.h"
> 
> Using double quotes should be fine for the current directory.
> 

No it is not enough. It not only gets included by this file, but also
gets included by include/trace/ftrace.h with:

#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)

Without that -I$(src) added, this include will fail.

-- Steve
Catalin Marinas Nov. 5, 2014, 3:52 p.m. UTC | #3
On Wed, Nov 05, 2014 at 03:05:11PM +0000, Steven Rostedt wrote:
> On Wed, 5 Nov 2014 14:46:19 +0000
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> > On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
> > > --- a/arch/arm64/kernel/Makefile
> > > +++ b/arch/arm64/kernel/Makefile
> > > @@ -5,6 +5,7 @@
> > >  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> > >  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> > >  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> > > +CFLAGS_armv8_deprecated.o := -I$(src)
> > 
> > Why do you need this?
> 
> It has to do with the TRACE_EVENT magic.
> 
> Read samples/trace_events/Makefile
> 
> > 
> > >  CFLAGS_REMOVE_ftrace.o = -pg
> > >  CFLAGS_REMOVE_insn.o = -pg
> > > diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> > > index fded15f..d376fe2 100644
> > > --- a/arch/arm64/kernel/armv8_deprecated.c
> > > +++ b/arch/arm64/kernel/armv8_deprecated.c
> > > @@ -15,6 +15,9 @@
> > >  #include <linux/slab.h>
> > >  #include <linux/sysctl.h>
> > >  
> > > +#define CREATE_TRACE_POINTS
> > > +#include "trace-events-emulation.h"
> > 
> > Using double quotes should be fine for the current directory.
> 
> No it is not enough. It not only gets included by this file, but also
> gets included by include/trace/ftrace.h with:
> 
> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> 
> Without that -I$(src) added, this include will fail.

Ah, thanks for clarification.
Will Deacon Nov. 10, 2014, 6:27 p.m. UTC | #4
On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
> Introduce an event to trace the usage of emulated instructions. The
> trace event is intended to help identify and encourage the migration
> of legacy software using the emulation features.
> 
> Use this event to trace usage of swp and CP15 barrier emulation.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  arch/arm64/kernel/Makefile                 |  1 +
>  arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
>  arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/kernel/trace-events-emulation.h
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 5362578..1fc7abd 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -5,6 +5,7 @@
>  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> +CFLAGS_armv8_deprecated.o := -I$(src)
>  
>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index fded15f..d376fe2 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -15,6 +15,9 @@
>  #include <linux/slab.h>
>  #include <linux/sysctl.h>
>  
> +#define CREATE_TRACE_POINTS
> +#include "trace-events-emulation.h"
> +
>  #include <asm/insn.h>
>  #include <asm/opcodes.h>
>  #include <asm/system_misc.h>
> @@ -203,6 +206,11 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>  		regs->user_regs.regs[destreg] = data;
>  
>  ret:
> +	if (type == TYPE_SWPB)
> +		trace_instruction_emulation("swpb", regs->pc);
> +	else
> +		trace_instruction_emulation("swp", regs->pc);
> +
>  	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
>  			current->comm, (unsigned long)current->pid, regs->pc);
>  
> @@ -260,16 +268,23 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>  		 * dmb - mcr p15, 0, Rt, c7, c10, 5
>  		 * dsb - mcr p15, 0, Rt, c7, c10, 4
>  		 */
> -		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
> +		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
>  			dmb(sy);
> -		else
> +			trace_instruction_emulation(
> +				"mcr p15, 0, Rt, c7, c10, 5", regs->pc);
> +		} else {
>  			dsb(sy);
> +			trace_instruction_emulation(
> +				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
> +		}
>  		break;
>  	case 5:
>  		/*
>  		 * isb - mcr p15, 0, Rt, c7, c5, 4
>  		 */
>  		isb();
> +		trace_instruction_emulation(
> +			"mcr p15, 0, Rt, c7, c5, 4", regs->pc);

Any chance you can put the barrier name in here too please? Maybe as an asm
comment, e.g.:

  "mcr p15, 0, Rt, c7, c5, 4 ; isb"

Will
Punit Agrawal Nov. 12, 2014, 11:31 a.m. UTC | #5
Hi Will,

Thanks for taking a look.

Will Deacon <will.deacon@arm.com> writes:

> On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
>> Introduce an event to trace the usage of emulated instructions. The
>> trace event is intended to help identify and encourage the migration
>> of legacy software using the emulation features.
>> 
>> Use this event to trace usage of swp and CP15 barrier emulation.
>> 
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> ---

[...]

>> @@ -260,16 +268,23 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>>  		 * dmb - mcr p15, 0, Rt, c7, c10, 5
>>  		 * dsb - mcr p15, 0, Rt, c7, c10, 4
>>  		 */
>> -		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
>> +		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
>>  			dmb(sy);
>> -		else
>> +			trace_instruction_emulation(
>> +				"mcr p15, 0, Rt, c7, c10, 5", regs->pc);
>> +		} else {
>>  			dsb(sy);
>> +			trace_instruction_emulation(
>> +				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
>> +		}
>>  		break;
>>  	case 5:
>>  		/*
>>  		 * isb - mcr p15, 0, Rt, c7, c5, 4
>>  		 */
>>  		isb();
>> +		trace_instruction_emulation(
>> +			"mcr p15, 0, Rt, c7, c5, 4", regs->pc);
>
> Any chance you can put the barrier name in here too please? Maybe as an asm
> comment, e.g.:
>
>   "mcr p15, 0, Rt, c7, c5, 4 ; isb"

Good point. I've added the barrier names as assembler comments.

Thanks.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 5362578..1fc7abd 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -5,6 +5,7 @@ 
 CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
+CFLAGS_armv8_deprecated.o := -I$(src)
 
 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index fded15f..d376fe2 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -15,6 +15,9 @@ 
 #include <linux/slab.h>
 #include <linux/sysctl.h>
 
+#define CREATE_TRACE_POINTS
+#include "trace-events-emulation.h"
+
 #include <asm/insn.h>
 #include <asm/opcodes.h>
 #include <asm/system_misc.h>
@@ -203,6 +206,11 @@  static int swp_handler(struct pt_regs *regs, u32 instr)
 		regs->user_regs.regs[destreg] = data;
 
 ret:
+	if (type == TYPE_SWPB)
+		trace_instruction_emulation("swpb", regs->pc);
+	else
+		trace_instruction_emulation("swp", regs->pc);
+
 	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
 			current->comm, (unsigned long)current->pid, regs->pc);
 
@@ -260,16 +268,23 @@  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
 		 * dmb - mcr p15, 0, Rt, c7, c10, 5
 		 * dsb - mcr p15, 0, Rt, c7, c10, 4
 		 */
-		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
+		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
 			dmb(sy);
-		else
+			trace_instruction_emulation(
+				"mcr p15, 0, Rt, c7, c10, 5", regs->pc);
+		} else {
 			dsb(sy);
+			trace_instruction_emulation(
+				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
+		}
 		break;
 	case 5:
 		/*
 		 * isb - mcr p15, 0, Rt, c7, c5, 4
 		 */
 		isb();
+		trace_instruction_emulation(
+			"mcr p15, 0, Rt, c7, c5, 4", regs->pc);
 		break;
 	}
 
diff --git a/arch/arm64/kernel/trace-events-emulation.h b/arch/arm64/kernel/trace-events-emulation.h
new file mode 100644
index 0000000..73aa750
--- /dev/null
+++ b/arch/arm64/kernel/trace-events-emulation.h
@@ -0,0 +1,40 @@ 
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM emulation
+
+#if !defined(_TRACE_EMULATION_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EMULATION_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(instruction_emulation,
+
+	TP_PROTO(const char *instr, u64 addr),
+	TP_ARGS(instr, addr),
+
+	TP_STRUCT__entry(
+		__array(char, comm, TASK_COMM_LEN)
+		__field(pid_t, pid)
+		__string(instr, instr)
+		__field(u64, addr)
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+		__entry->pid = current->pid;
+		__assign_str(instr, instr);
+		__entry->addr = addr;
+	),
+
+	TP_printk("instr=%s comm=%s pid=%d addr=0x%llx", __get_str(instr),
+		__entry->comm, __entry->pid, __entry->addr)
+);
+
+#endif /* _TRACE_EMULATION_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+
+#define TRACE_INCLUDE_FILE trace-events-emulation
+#include <trace/define_trace.h>