[PATCH 0/2] Support bpf prologue for arm64

[PATCH 0/2] Support bpf prologue for arm64

[He Kuang]

Two patches here makes bpf prologue available for arm64.

He Kuang (2):
  perf probe: Fix wrong register name for arm64
  perf tools: Introduce regs_query_register_offset() for arm64

 tools/perf/arch/arm64/Makefile                   |   1 +
 tools/perf/arch/arm64/include/dwarf-regs-table.h |  12 +--
 tools/perf/arch/arm64/util/dwarf-regs.c          | 123 +++++++++++++----------
 3 files changed, 77 insertions(+), 59 deletions(-)

-- 
1.8.5.2

[PATCH 1/2] perf probe: Fix wrong register name for arm64

[He Kuang]

The register name of arm64 architecture is x0-x31 not r0-r31, this
patch changes this typo.

Before this patch:

  # perf probe --definition 'sys_write count'
  p:probe/sys_write _text+1502872 count=%r2:s64

  # echo 'p:probe/sys_write _text+1502872 count=%r2:s64' > \
    /sys/kernel/debug/tracing/kprobe_events
  Parse error at argument[0]. (-22)

After this patch:

  # perf probe --definition 'sys_write count'
  p:probe/sys_write _text+1502872 count=%x2:s64

  # echo 'p:probe/sys_write _text+1502872 count=%x2:s64' > \
    /sys/kernel/debug/tracing/kprobe_events
  # echo 1 >/sys/kernel/debug/tracing/events/probe/enable
  # cat /sys/kernel/debug/tracing/trace
  ...
  sh-422   [000] d... 650.495930: sys_write: (SyS_write+0x0/0xc8) count=22
  sh-422   [000] d... 651.102389: sys_write: (SyS_write+0x0/0xc8) count=26
  sh-422   [000] d... 651.358653: sys_write: (SyS_write+0x0/0xc8) count=86

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/arm64/include/dwarf-regs-table.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
index 2675936..36e375f 100644
--- a/tools/perf/arch/arm64/include/dwarf-regs-table.h
+++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
@@ -2,12 +2,12 @@
 /* This is included in perf/util/dwarf-regs.c */
 
 static const char * const aarch64_regstr_tbl[] = {
-	"%r0", "%r1", "%r2", "%r3", "%r4",
-	"%r5", "%r6", "%r7", "%r8", "%r9",
-	"%r10", "%r11", "%r12", "%r13", "%r14",
-	"%r15", "%r16", "%r17", "%r18", "%r19",
-	"%r20", "%r21", "%r22", "%r23", "%r24",
-	"%r25", "%r26", "%r27", "%r28", "%r29",
+	"%x0", "%x1", "%x2", "%x3", "%x4",
+	"%x5", "%x6", "%x7", "%x8", "%x9",
+	"%x10", "%x11", "%x12", "%x13", "%x14",
+	"%x15", "%x16", "%x17", "%x18", "%x19",
+	"%x20", "%x21", "%x22", "%x23", "%x24",
+	"%x25", "%x26", "%x27", "%x28", "%x29",
 	"%lr", "%sp",
 };
 #endif
-- 
1.8.5.2

[PATCH 2/2] perf tools: Introduce regs_query_register_offset() for arm64

[He Kuang]

Since HAVE_KPROBES can be enabled in arm64, this patch introduces
regs_query_register_offset() to convert register name to offset for
arm64, so the BPF prologue feature is ready to use.

This patch also changes the 'dwarfnum' to 'offset' in register table,
so the related functions are consistent with x86.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/arm64/Makefile          |   1 +
 tools/perf/arch/arm64/util/dwarf-regs.c | 123 ++++++++++++++++++--------------
 2 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 18b1351..eebe1ec 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -2,3 +2,4 @@ ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
 PERF_HAVE_JITDUMP := 1
+PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
index d49efeb..6427ce0 100644
--- a/tools/perf/arch/arm64/util/dwarf-regs.c
+++ b/tools/perf/arch/arm64/util/dwarf-regs.c
@@ -9,72 +9,89 @@
  */
 
 #include <stddef.h>
+#include <linux/ptrace.h> /* for struct user_pt_regs */
+#include <errno.h> /* for EINVAL */
+#include <string.h> /* for strcmp */
 #include <dwarf-regs.h>
 
-struct pt_regs_dwarfnum {
+struct pt_regs_offset {
 	const char *name;
-	unsigned int dwarfnum;
+	int offset;
 };
 
-#define STR(s) #s
-#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
-#define GPR_DWARFNUM_NAME(num) \
-	{.name = STR(%x##num), .dwarfnum = num}
-#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
-
 /*
  * Reference:
  * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
  */
-static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
-	GPR_DWARFNUM_NAME(0),
-	GPR_DWARFNUM_NAME(1),
-	GPR_DWARFNUM_NAME(2),
-	GPR_DWARFNUM_NAME(3),
-	GPR_DWARFNUM_NAME(4),
-	GPR_DWARFNUM_NAME(5),
-	GPR_DWARFNUM_NAME(6),
-	GPR_DWARFNUM_NAME(7),
-	GPR_DWARFNUM_NAME(8),
-	GPR_DWARFNUM_NAME(9),
-	GPR_DWARFNUM_NAME(10),
-	GPR_DWARFNUM_NAME(11),
-	GPR_DWARFNUM_NAME(12),
-	GPR_DWARFNUM_NAME(13),
-	GPR_DWARFNUM_NAME(14),
-	GPR_DWARFNUM_NAME(15),
-	GPR_DWARFNUM_NAME(16),
-	GPR_DWARFNUM_NAME(17),
-	GPR_DWARFNUM_NAME(18),
-	GPR_DWARFNUM_NAME(19),
-	GPR_DWARFNUM_NAME(20),
-	GPR_DWARFNUM_NAME(21),
-	GPR_DWARFNUM_NAME(22),
-	GPR_DWARFNUM_NAME(23),
-	GPR_DWARFNUM_NAME(24),
-	GPR_DWARFNUM_NAME(25),
-	GPR_DWARFNUM_NAME(26),
-	GPR_DWARFNUM_NAME(27),
-	GPR_DWARFNUM_NAME(28),
-	GPR_DWARFNUM_NAME(29),
-	REG_DWARFNUM_NAME("%lr", 30),
-	REG_DWARFNUM_NAME("%sp", 31),
-	REG_DWARFNUM_END,
+#define REG_OFFSET_NAME(r) {.name = #r,		\
+			.offset = offsetof(struct user_pt_regs, r)}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+#define GPR_OFFSET_NAME(r) \
+	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
+
+static const struct pt_regs_offset regoffset_table[] = {
+	GPR_OFFSET_NAME(0),
+	GPR_OFFSET_NAME(1),
+	GPR_OFFSET_NAME(2),
+	GPR_OFFSET_NAME(3),
+	GPR_OFFSET_NAME(4),
+	GPR_OFFSET_NAME(5),
+	GPR_OFFSET_NAME(6),
+	GPR_OFFSET_NAME(7),
+	GPR_OFFSET_NAME(8),
+	GPR_OFFSET_NAME(9),
+	GPR_OFFSET_NAME(10),
+	GPR_OFFSET_NAME(11),
+	GPR_OFFSET_NAME(12),
+	GPR_OFFSET_NAME(13),
+	GPR_OFFSET_NAME(14),
+	GPR_OFFSET_NAME(15),
+	GPR_OFFSET_NAME(16),
+	GPR_OFFSET_NAME(17),
+	GPR_OFFSET_NAME(18),
+	GPR_OFFSET_NAME(19),
+	GPR_OFFSET_NAME(20),
+	GPR_OFFSET_NAME(21),
+	GPR_OFFSET_NAME(22),
+	GPR_OFFSET_NAME(23),
+	GPR_OFFSET_NAME(24),
+	GPR_OFFSET_NAME(25),
+	GPR_OFFSET_NAME(26),
+	GPR_OFFSET_NAME(27),
+	GPR_OFFSET_NAME(28),
+	GPR_OFFSET_NAME(29),
+	GPR_OFFSET_NAME(30),
+	{.name = "lr", .offset = offsetof(struct user_pt_regs, regs[30])},
+	REG_OFFSET_NAME(sp),
+	REG_OFFSET_NAME(pc),
+	REG_OFFSET_NAME(pstate),
+	REG_OFFSET_END,
 };
 
+/* Minus 1 for the ending REG_OFFSET_END */
+#define ARCH_MAX_REGS ((sizeof(regoffset_table) /		\
+			sizeof(regoffset_table[0])) - 1)
+
+/* Return architecture dependent register string (for kprobe-tracer) */
+const char *get_arch_regstr(unsigned int n)
+{
+	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
+}
+
+/* Reuse code from arch/arm64/kernel/ptrace.c */
 /**
- * get_arch_regstr() - lookup register name from it's DWARF register number
- * @n:	the DWARF register number
+ * regs_query_register_offset() - query register offset from its name
+ * @name:	the name of a register
  *
- * get_arch_regstr() returns the name of the register in struct
- * regdwarfnum_table from it's DWARF register number. If the register is not
- * found in the table, this returns NULL;
+ * regs_query_register_offset() returns the offset of a register in struct
+ * user_pt_regs from its name. If the name is invalid, this returns -EINVAL;
  */
-const char *get_arch_regstr(unsigned int n)
+int regs_query_register_offset(const char *name)
 {
-	const struct pt_regs_dwarfnum *roff;
-	for (roff = regdwarfnum_table; roff->name != NULL; roff++)
-		if (roff->dwarfnum == n)
-			return roff->name;
-	return NULL;
+	const struct pt_regs_offset *roff;
+
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (!strcmp(roff->name, name))
+			return roff->offset;
+	return -EINVAL;
 }
-- 
1.8.5.2

Re: [PATCH 0/2] Support bpf prologue for arm64

[Arnaldo Carvalho de Melo]

Em Tue, Jan 24, 2017 at 10:30:13AM +0000, He Kuang escreveu:
> Two patches here makes bpf prologue available for arm64.

Thanks, looks good, applied,

- Arnaldo
 
> He Kuang (2):
>   perf probe: Fix wrong register name for arm64
>   perf tools: Introduce regs_query_register_offset() for arm64
> 
>  tools/perf/arch/arm64/Makefile                   |   1 +
>  tools/perf/arch/arm64/include/dwarf-regs-table.h |  12 +--
>  tools/perf/arch/arm64/util/dwarf-regs.c          | 123 +++++++++++++----------
>  3 files changed, 77 insertions(+), 59 deletions(-)
> 
> -- 
> 1.8.5.2

Re: [PATCH 2/2] perf tools: Introduce regs_query_register_offset() for arm64

[Will Deacon]

On Tue, Jan 24, 2017 at 10:30:15AM +0000, He Kuang wrote:
> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> regs_query_register_offset() to convert register name to offset for
> arm64, so the BPF prologue feature is ready to use.
> 
> This patch also changes the 'dwarfnum' to 'offset' in register table,
> so the related functions are consistent with x86.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/Makefile          |   1 +
>  tools/perf/arch/arm64/util/dwarf-regs.c | 123 ++++++++++++++++++--------------
>  2 files changed, 71 insertions(+), 53 deletions(-)

It would've been nice to have been cc'd on this. In future, please at least
cc linux-arm-kernel for patches directly changing arm/arm64 code.

> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
> index 18b1351..eebe1ec 100644
> --- a/tools/perf/arch/arm64/Makefile
> +++ b/tools/perf/arch/arm64/Makefile
> @@ -2,3 +2,4 @@ ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
>  PERF_HAVE_JITDUMP := 1
> +PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
> diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
> index d49efeb..6427ce0 100644
> --- a/tools/perf/arch/arm64/util/dwarf-regs.c
> +++ b/tools/perf/arch/arm64/util/dwarf-regs.c
> @@ -9,72 +9,89 @@
>   */
>  
>  #include <stddef.h>
> +#include <linux/ptrace.h> /* for struct user_pt_regs */
> +#include <errno.h> /* for EINVAL */
> +#include <string.h> /* for strcmp */
>  #include <dwarf-regs.h>
>  
> -struct pt_regs_dwarfnum {
> +struct pt_regs_offset {
>  	const char *name;
> -	unsigned int dwarfnum;
> +	int offset;
>  };
>  
> -#define STR(s) #s
> -#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
> -#define GPR_DWARFNUM_NAME(num) \
> -	{.name = STR(%x##num), .dwarfnum = num}
> -#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
> -
>  /*
>   * Reference:
>   * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
>   */
> -static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
> -	GPR_DWARFNUM_NAME(0),
> -	GPR_DWARFNUM_NAME(1),
> -	GPR_DWARFNUM_NAME(2),
> -	GPR_DWARFNUM_NAME(3),
> -	GPR_DWARFNUM_NAME(4),
> -	GPR_DWARFNUM_NAME(5),
> -	GPR_DWARFNUM_NAME(6),
> -	GPR_DWARFNUM_NAME(7),
> -	GPR_DWARFNUM_NAME(8),
> -	GPR_DWARFNUM_NAME(9),
> -	GPR_DWARFNUM_NAME(10),
> -	GPR_DWARFNUM_NAME(11),
> -	GPR_DWARFNUM_NAME(12),
> -	GPR_DWARFNUM_NAME(13),
> -	GPR_DWARFNUM_NAME(14),
> -	GPR_DWARFNUM_NAME(15),
> -	GPR_DWARFNUM_NAME(16),
> -	GPR_DWARFNUM_NAME(17),
> -	GPR_DWARFNUM_NAME(18),
> -	GPR_DWARFNUM_NAME(19),
> -	GPR_DWARFNUM_NAME(20),
> -	GPR_DWARFNUM_NAME(21),
> -	GPR_DWARFNUM_NAME(22),
> -	GPR_DWARFNUM_NAME(23),
> -	GPR_DWARFNUM_NAME(24),
> -	GPR_DWARFNUM_NAME(25),
> -	GPR_DWARFNUM_NAME(26),
> -	GPR_DWARFNUM_NAME(27),
> -	GPR_DWARFNUM_NAME(28),
> -	GPR_DWARFNUM_NAME(29),
> -	REG_DWARFNUM_NAME("%lr", 30),
> -	REG_DWARFNUM_NAME("%sp", 31),
> -	REG_DWARFNUM_END,
> +#define REG_OFFSET_NAME(r) {.name = #r,		\
> +			.offset = offsetof(struct user_pt_regs, r)}
> +#define REG_OFFSET_END {.name = NULL, .offset = 0}
> +#define GPR_OFFSET_NAME(r) \
> +	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
> +
> +static const struct pt_regs_offset regoffset_table[] = {
> +	GPR_OFFSET_NAME(0),
> +	GPR_OFFSET_NAME(1),
> +	GPR_OFFSET_NAME(2),
> +	GPR_OFFSET_NAME(3),
> +	GPR_OFFSET_NAME(4),
> +	GPR_OFFSET_NAME(5),
> +	GPR_OFFSET_NAME(6),
> +	GPR_OFFSET_NAME(7),
> +	GPR_OFFSET_NAME(8),
> +	GPR_OFFSET_NAME(9),
> +	GPR_OFFSET_NAME(10),
> +	GPR_OFFSET_NAME(11),
> +	GPR_OFFSET_NAME(12),
> +	GPR_OFFSET_NAME(13),
> +	GPR_OFFSET_NAME(14),
> +	GPR_OFFSET_NAME(15),
> +	GPR_OFFSET_NAME(16),
> +	GPR_OFFSET_NAME(17),
> +	GPR_OFFSET_NAME(18),
> +	GPR_OFFSET_NAME(19),
> +	GPR_OFFSET_NAME(20),
> +	GPR_OFFSET_NAME(21),
> +	GPR_OFFSET_NAME(22),
> +	GPR_OFFSET_NAME(23),
> +	GPR_OFFSET_NAME(24),
> +	GPR_OFFSET_NAME(25),
> +	GPR_OFFSET_NAME(26),
> +	GPR_OFFSET_NAME(27),
> +	GPR_OFFSET_NAME(28),
> +	GPR_OFFSET_NAME(29),
> +	GPR_OFFSET_NAME(30),
> +	{.name = "lr", .offset = offsetof(struct user_pt_regs, regs[30])},
> +	REG_OFFSET_NAME(sp),

Don't sp and lr need the leading '%'?

> +	REG_OFFSET_NAME(pc),
> +	REG_OFFSET_NAME(pstate),

The AArch64 DWARF spec says that DWARF register 32 is "RESERVED" and
register 33 is the ELR, so these pc/pstate entries are wrong.

However, with those changes, I think this patch can simply be ignored and
mainline is doing the right thing.

Will

Re: [PATCH 2/2] perf tools: Introduce regs_query_register_offset() for arm64

[Arnaldo Carvalho de Melo]

Em Tue, Jan 24, 2017 at 06:25:18PM +0000, Will Deacon escreveu:
> On Tue, Jan 24, 2017 at 10:30:15AM +0000, He Kuang wrote:
> > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > regs_query_register_offset() to convert register name to offset for
> > arm64, so the BPF prologue feature is ready to use.
> > 
> > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > so the related functions are consistent with x86.
> > 
> > Signed-off-by: He Kuang <hekuang@huawei.com>
 
> It would've been nice to have been cc'd on this. In future, please at least
> cc linux-arm-kernel for patches directly changing arm/arm64 code.
 
> > +	GPR_OFFSET_NAME(30),
> > +	{.name = "lr", .offset = offsetof(struct user_pt_regs, regs[30])},
> > +	REG_OFFSET_NAME(sp),
 
> Don't sp and lr need the leading '%'?
 
> > +	REG_OFFSET_NAME(pc),
> > +	REG_OFFSET_NAME(pstate),
 
> The AArch64 DWARF spec says that DWARF register 32 is "RESERVED" and
> register 33 is the ELR, so these pc/pstate entries are wrong.
 
> However, with those changes, I think this patch can simply be ignored and
> mainline is doing the right thing.

Ok, thanks for checking, dropping this patch then.

- Arnaldo

Re: [PATCH 1/2] perf probe: Fix wrong register name for arm64

[Arnaldo Carvalho de Melo]

Em Tue, Jan 24, 2017 at 10:30:14AM +0000, He Kuang escreveu:
> The register name of arm64 architecture is x0-x31 not r0-r31, this
> patch changes this typo.
> 
> Before this patch:

Will, are you ok with this one? I'll make sure I'll forward patches in
this area to you if you're not on the CC list,

- Arnaldo

>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%r2:s64
> 
>   # echo 'p:probe/sys_write _text+1502872 count=%r2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   Parse error at argument[0]. (-22)
> 
> After this patch:
> 
>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%x2:s64
> 
>   # echo 'p:probe/sys_write _text+1502872 count=%x2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   # echo 1 >/sys/kernel/debug/tracing/events/probe/enable
>   # cat /sys/kernel/debug/tracing/trace
>   ...
>   sh-422   [000] d... 650.495930: sys_write: (SyS_write+0x0/0xc8) count=22
>   sh-422   [000] d... 651.102389: sys_write: (SyS_write+0x0/0xc8) count=26
>   sh-422   [000] d... 651.358653: sys_write: (SyS_write+0x0/0xc8) count=86
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/include/dwarf-regs-table.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> index 2675936..36e375f 100644
> --- a/tools/perf/arch/arm64/include/dwarf-regs-table.h
> +++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> @@ -2,12 +2,12 @@
>  /* This is included in perf/util/dwarf-regs.c */
>  
>  static const char * const aarch64_regstr_tbl[] = {
> -	"%r0", "%r1", "%r2", "%r3", "%r4",
> -	"%r5", "%r6", "%r7", "%r8", "%r9",
> -	"%r10", "%r11", "%r12", "%r13", "%r14",
> -	"%r15", "%r16", "%r17", "%r18", "%r19",
> -	"%r20", "%r21", "%r22", "%r23", "%r24",
> -	"%r25", "%r26", "%r27", "%r28", "%r29",
> +	"%x0", "%x1", "%x2", "%x3", "%x4",
> +	"%x5", "%x6", "%x7", "%x8", "%x9",
> +	"%x10", "%x11", "%x12", "%x13", "%x14",
> +	"%x15", "%x16", "%x17", "%x18", "%x19",
> +	"%x20", "%x21", "%x22", "%x23", "%x24",
> +	"%x25", "%x26", "%x27", "%x28", "%x29",
>  	"%lr", "%sp",
>  };
>  #endif
> -- 
> 1.8.5.2

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[He Kuang]

Since HAVE_KPROBES can be enabled in arm64, this patch introduces
regs_query_register_offset() to convert register name to offset for
arm64, so the BPF prologue feature is ready to use.

This patch also changes the 'dwarfnum' to 'offset' in register table,
so the related functions are consistent with x86.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/arm64/Makefile          |   1 +
 tools/perf/arch/arm64/util/dwarf-regs.c | 124 ++++++++++++++++++--------------
 2 files changed, 72 insertions(+), 53 deletions(-)

diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 18b1351..eebe1ec 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -2,3 +2,4 @@ ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
 PERF_HAVE_JITDUMP := 1
+PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
index d49efeb..5ec62a4 100644
--- a/tools/perf/arch/arm64/util/dwarf-regs.c
+++ b/tools/perf/arch/arm64/util/dwarf-regs.c
@@ -9,72 +9,90 @@
  */
 
 #include <stddef.h>
+#include <linux/ptrace.h> /* for struct user_pt_regs */
+#include <errno.h> /* for EINVAL */
+#include <string.h> /* for strcmp */
+#include <linux/ptrace.h> /* for struct user_pt_regs */
 #include <dwarf-regs.h>
 
-struct pt_regs_dwarfnum {
+struct pt_regs_offset {
 	const char *name;
-	unsigned int dwarfnum;
+	int offset;
 };
 
-#define STR(s) #s
-#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
-#define GPR_DWARFNUM_NAME(num) \
-	{.name = STR(%x##num), .dwarfnum = num}
-#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
-
 /*
  * Reference:
  * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
  */
-static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
-	GPR_DWARFNUM_NAME(0),
-	GPR_DWARFNUM_NAME(1),
-	GPR_DWARFNUM_NAME(2),
-	GPR_DWARFNUM_NAME(3),
-	GPR_DWARFNUM_NAME(4),
-	GPR_DWARFNUM_NAME(5),
-	GPR_DWARFNUM_NAME(6),
-	GPR_DWARFNUM_NAME(7),
-	GPR_DWARFNUM_NAME(8),
-	GPR_DWARFNUM_NAME(9),
-	GPR_DWARFNUM_NAME(10),
-	GPR_DWARFNUM_NAME(11),
-	GPR_DWARFNUM_NAME(12),
-	GPR_DWARFNUM_NAME(13),
-	GPR_DWARFNUM_NAME(14),
-	GPR_DWARFNUM_NAME(15),
-	GPR_DWARFNUM_NAME(16),
-	GPR_DWARFNUM_NAME(17),
-	GPR_DWARFNUM_NAME(18),
-	GPR_DWARFNUM_NAME(19),
-	GPR_DWARFNUM_NAME(20),
-	GPR_DWARFNUM_NAME(21),
-	GPR_DWARFNUM_NAME(22),
-	GPR_DWARFNUM_NAME(23),
-	GPR_DWARFNUM_NAME(24),
-	GPR_DWARFNUM_NAME(25),
-	GPR_DWARFNUM_NAME(26),
-	GPR_DWARFNUM_NAME(27),
-	GPR_DWARFNUM_NAME(28),
-	GPR_DWARFNUM_NAME(29),
-	REG_DWARFNUM_NAME("%lr", 30),
-	REG_DWARFNUM_NAME("%sp", 31),
-	REG_DWARFNUM_END,
+#define REG_OFFSET_NAME(r, num) {.name = "%" #r,			\
+			.offset = offsetof(struct user_pt_regs, regs[num])}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+#define GPR_OFFSET_NAME(r) \
+	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
+
+/* This table is for reverse searching for the offset or register
+ * names in aarch64_regstr_tbl[].
+ */
+static const struct pt_regs_offset regoffset_table[] = {
+	GPR_OFFSET_NAME(0),
+	GPR_OFFSET_NAME(1),
+	GPR_OFFSET_NAME(2),
+	GPR_OFFSET_NAME(3),
+	GPR_OFFSET_NAME(4),
+	GPR_OFFSET_NAME(5),
+	GPR_OFFSET_NAME(6),
+	GPR_OFFSET_NAME(7),
+	GPR_OFFSET_NAME(8),
+	GPR_OFFSET_NAME(9),
+	GPR_OFFSET_NAME(10),
+	GPR_OFFSET_NAME(11),
+	GPR_OFFSET_NAME(12),
+	GPR_OFFSET_NAME(13),
+	GPR_OFFSET_NAME(14),
+	GPR_OFFSET_NAME(15),
+	GPR_OFFSET_NAME(16),
+	GPR_OFFSET_NAME(17),
+	GPR_OFFSET_NAME(18),
+	GPR_OFFSET_NAME(19),
+	GPR_OFFSET_NAME(20),
+	GPR_OFFSET_NAME(21),
+	GPR_OFFSET_NAME(22),
+	GPR_OFFSET_NAME(23),
+	GPR_OFFSET_NAME(24),
+	GPR_OFFSET_NAME(25),
+	GPR_OFFSET_NAME(26),
+	GPR_OFFSET_NAME(27),
+	GPR_OFFSET_NAME(28),
+	GPR_OFFSET_NAME(29),
+	REG_OFFSET_NAME(lr, 30),
+	REG_OFFSET_NAME(sp, 31),
+	REG_OFFSET_END,
 };
 
+/* Minus 1 for the ending REG_OFFSET_END */
+#define ARCH_MAX_REGS ((sizeof(regoffset_table) /		\
+			sizeof(regoffset_table[0])) - 1)
+
+/* Return architecture dependent register string (for kprobe-tracer) */
+const char *get_arch_regstr(unsigned int n)
+{
+	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
+}
+
+/* Reuse code from arch/arm64/kernel/ptrace.c */
 /**
- * get_arch_regstr() - lookup register name from it's DWARF register number
- * @n:	the DWARF register number
+ * regs_query_register_offset() - query register offset from its name
+ * @name:	the name of a register
  *
- * get_arch_regstr() returns the name of the register in struct
- * regdwarfnum_table from it's DWARF register number. If the register is not
- * found in the table, this returns NULL;
+ * regs_query_register_offset() returns the offset of a register in struct
+ * user_pt_regs from its name. If the name is invalid, this returns -EINVAL;
  */
-const char *get_arch_regstr(unsigned int n)
+int regs_query_register_offset(const char *name)
 {
-	const struct pt_regs_dwarfnum *roff;
-	for (roff = regdwarfnum_table; roff->name != NULL; roff++)
-		if (roff->dwarfnum == n)
-			return roff->name;
-	return NULL;
+	const struct pt_regs_offset *roff;
+
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (!strcmp(roff->name, name))
+			return roff->offset;
+	return -EINVAL;
 }
-- 
1.8.5.2

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[He Kuang]

Since HAVE_KPROBES can be enabled in arm64, this patch introduces
regs_query_register_offset() to convert register name to offset for
arm64, so the BPF prologue feature is ready to use.

This patch also changes the 'dwarfnum' to 'offset' in register table,
so the related functions are consistent with x86.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/arch/arm64/Makefile          |   1 +
 tools/perf/arch/arm64/util/dwarf-regs.c | 124 ++++++++++++++++++--------------
 2 files changed, 72 insertions(+), 53 deletions(-)

diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 18b1351..eebe1ec 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -2,3 +2,4 @@ ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
 PERF_HAVE_JITDUMP := 1
+PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
index d49efeb..5ec62a4 100644
--- a/tools/perf/arch/arm64/util/dwarf-regs.c
+++ b/tools/perf/arch/arm64/util/dwarf-regs.c
@@ -9,72 +9,90 @@
  */
 
 #include <stddef.h>
+#include <linux/ptrace.h> /* for struct user_pt_regs */
+#include <errno.h> /* for EINVAL */
+#include <string.h> /* for strcmp */
+#include <linux/ptrace.h> /* for struct user_pt_regs */
 #include <dwarf-regs.h>
 
-struct pt_regs_dwarfnum {
+struct pt_regs_offset {
 	const char *name;
-	unsigned int dwarfnum;
+	int offset;
 };
 
-#define STR(s) #s
-#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
-#define GPR_DWARFNUM_NAME(num) \
-	{.name = STR(%x##num), .dwarfnum = num}
-#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
-
 /*
  * Reference:
  * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
  */
-static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
-	GPR_DWARFNUM_NAME(0),
-	GPR_DWARFNUM_NAME(1),
-	GPR_DWARFNUM_NAME(2),
-	GPR_DWARFNUM_NAME(3),
-	GPR_DWARFNUM_NAME(4),
-	GPR_DWARFNUM_NAME(5),
-	GPR_DWARFNUM_NAME(6),
-	GPR_DWARFNUM_NAME(7),
-	GPR_DWARFNUM_NAME(8),
-	GPR_DWARFNUM_NAME(9),
-	GPR_DWARFNUM_NAME(10),
-	GPR_DWARFNUM_NAME(11),
-	GPR_DWARFNUM_NAME(12),
-	GPR_DWARFNUM_NAME(13),
-	GPR_DWARFNUM_NAME(14),
-	GPR_DWARFNUM_NAME(15),
-	GPR_DWARFNUM_NAME(16),
-	GPR_DWARFNUM_NAME(17),
-	GPR_DWARFNUM_NAME(18),
-	GPR_DWARFNUM_NAME(19),
-	GPR_DWARFNUM_NAME(20),
-	GPR_DWARFNUM_NAME(21),
-	GPR_DWARFNUM_NAME(22),
-	GPR_DWARFNUM_NAME(23),
-	GPR_DWARFNUM_NAME(24),
-	GPR_DWARFNUM_NAME(25),
-	GPR_DWARFNUM_NAME(26),
-	GPR_DWARFNUM_NAME(27),
-	GPR_DWARFNUM_NAME(28),
-	GPR_DWARFNUM_NAME(29),
-	REG_DWARFNUM_NAME("%lr", 30),
-	REG_DWARFNUM_NAME("%sp", 31),
-	REG_DWARFNUM_END,
+#define REG_OFFSET_NAME(r, num) {.name = "%" #r,			\
+			.offset = offsetof(struct user_pt_regs, regs[num])}
+#define REG_OFFSET_END {.name = NULL, .offset = 0}
+#define GPR_OFFSET_NAME(r) \
+	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
+
+/* This table is for reverse searching for the offset or register
+ * names in aarch64_regstr_tbl[].
+ */
+static const struct pt_regs_offset regoffset_table[] = {
+	GPR_OFFSET_NAME(0),
+	GPR_OFFSET_NAME(1),
+	GPR_OFFSET_NAME(2),
+	GPR_OFFSET_NAME(3),
+	GPR_OFFSET_NAME(4),
+	GPR_OFFSET_NAME(5),
+	GPR_OFFSET_NAME(6),
+	GPR_OFFSET_NAME(7),
+	GPR_OFFSET_NAME(8),
+	GPR_OFFSET_NAME(9),
+	GPR_OFFSET_NAME(10),
+	GPR_OFFSET_NAME(11),
+	GPR_OFFSET_NAME(12),
+	GPR_OFFSET_NAME(13),
+	GPR_OFFSET_NAME(14),
+	GPR_OFFSET_NAME(15),
+	GPR_OFFSET_NAME(16),
+	GPR_OFFSET_NAME(17),
+	GPR_OFFSET_NAME(18),
+	GPR_OFFSET_NAME(19),
+	GPR_OFFSET_NAME(20),
+	GPR_OFFSET_NAME(21),
+	GPR_OFFSET_NAME(22),
+	GPR_OFFSET_NAME(23),
+	GPR_OFFSET_NAME(24),
+	GPR_OFFSET_NAME(25),
+	GPR_OFFSET_NAME(26),
+	GPR_OFFSET_NAME(27),
+	GPR_OFFSET_NAME(28),
+	GPR_OFFSET_NAME(29),
+	REG_OFFSET_NAME(lr, 30),
+	REG_OFFSET_NAME(sp, 31),
+	REG_OFFSET_END,
 };
 
+/* Minus 1 for the ending REG_OFFSET_END */
+#define ARCH_MAX_REGS ((sizeof(regoffset_table) /		\
+			sizeof(regoffset_table[0])) - 1)
+
+/* Return architecture dependent register string (for kprobe-tracer) */
+const char *get_arch_regstr(unsigned int n)
+{
+	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
+}
+
+/* Reuse code from arch/arm64/kernel/ptrace.c */
 /**
- * get_arch_regstr() - lookup register name from it's DWARF register number
- * @n:	the DWARF register number
+ * regs_query_register_offset() - query register offset from its name
+ * @name:	the name of a register
  *
- * get_arch_regstr() returns the name of the register in struct
- * regdwarfnum_table from it's DWARF register number. If the register is not
- * found in the table, this returns NULL;
+ * regs_query_register_offset() returns the offset of a register in struct
+ * user_pt_regs from its name. If the name is invalid, this returns -EINVAL;
  */
-const char *get_arch_regstr(unsigned int n)
+int regs_query_register_offset(const char *name)
 {
-	const struct pt_regs_dwarfnum *roff;
-	for (roff = regdwarfnum_table; roff->name != NULL; roff++)
-		if (roff->dwarfnum == n)
-			return roff->name;
-	return NULL;
+	const struct pt_regs_offset *roff;
+
+	for (roff = regoffset_table; roff->name != NULL; roff++)
+		if (!strcmp(roff->name, name))
+			return roff->offset;
+	return -EINVAL;
 }
-- 
1.8.5.2

Re: [PATCH 2/2] perf tools: Introduce regs_query_register_offset() for arm64

[Hekuang]

hi

在 2017/1/25 3:09, Arnaldo Carvalho de Melo 写道:
> Em Tue, Jan 24, 2017 at 06:25:18PM +0000, Will Deacon escreveu:
>> On Tue, Jan 24, 2017 at 10:30:15AM +0000, He Kuang wrote:
>>> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
>>> regs_query_register_offset() to convert register name to offset for
>>> arm64, so the BPF prologue feature is ready to use.
>>>
>>> This patch also changes the 'dwarfnum' to 'offset' in register table,
>>> so the related functions are consistent with x86.
>>>
>>> Signed-off-by: He Kuang <hekuang@huawei.com>
>   
>> It would've been nice to have been cc'd on this. In future, please at least
>> cc linux-arm-kernel for patches directly changing arm/arm64 code.
>   
>>> +	GPR_OFFSET_NAME(30),
>>> +	{.name = "lr", .offset = offsetof(struct user_pt_regs, regs[30])},
>>> +	REG_OFFSET_NAME(sp),
>   
>> Don't sp and lr need the leading '%'?
>   
>>> +	REG_OFFSET_NAME(pc),
>>> +	REG_OFFSET_NAME(pstate),
>   
>> The AArch64 DWARF spec says that DWARF register 32 is "RESERVED" and
>> register 33 is the ELR, so these pc/pstate entries are wrong.
>   
>> However, with those changes, I think this patch can simply be ignored and
>> mainline is doing the right thing.
> Ok, thanks for checking, dropping this patch then.
>
> - Arnaldo
>

The purpose of this patch is mainly on enable bpf prologue on arm64,
a new v2 version is sent and fix the problem mentioned by Will, thank
you for reviewing this.

Re: [PATCH 1/2] perf probe: Fix wrong register name for arm64

[Will Deacon]

On Tue, Jan 24, 2017 at 04:11:11PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 24, 2017 at 10:30:14AM +0000, He Kuang escreveu:
> > The register name of arm64 architecture is x0-x31 not r0-r31, this
> > patch changes this typo.
> > 
> > Before this patch:
> 
> Will, are you ok with this one? I'll make sure I'll forward patches in
> this area to you if you're not on the CC list,

Thanks, Arnaldo. Yes, this one looks correct to me:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Will Deacon]

On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> regs_query_register_offset() to convert register name to offset for
> arm64, so the BPF prologue feature is ready to use.
> 
> This patch also changes the 'dwarfnum' to 'offset' in register table,
> so the related functions are consistent with x86.

Wouldn't it be an awful lot simpler just to leave the code as-is, and
implement regs_query_register_offset in the same way that we implement
get_arch_regstr but return the dwarfnum?

I don't really see the point of all the refactoring.

Will

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Will Deacon]

On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> regs_query_register_offset() to convert register name to offset for
> arm64, so the BPF prologue feature is ready to use.
> 
> This patch also changes the 'dwarfnum' to 'offset' in register table,
> so the related functions are consistent with x86.

Wouldn't it be an awful lot simpler just to leave the code as-is, and
implement regs_query_register_offset in the same way that we implement
get_arch_regstr but return the dwarfnum?

I don't really see the point of all the refactoring.

Will

Re: [PATCH 1/2] perf probe: Fix wrong register name for arm64

[Masami Hiramatsu]

On Tue, 24 Jan 2017 10:30:14 +0000
He Kuang <hekuang@huawei.com> wrote:

> The register name of arm64 architecture is x0-x31 not r0-r31, this
> patch changes this typo.
> 
> Before this patch:
> 
>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%r2:s64
> 
>   # echo 'p:probe/sys_write _text+1502872 count=%r2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   Parse error at argument[0]. (-22)
> 
> After this patch:
> 
>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%x2:s64
> 
>   # echo 'p:probe/sys_write _text+1502872 count=%x2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   # echo 1 >/sys/kernel/debug/tracing/events/probe/enable
>   # cat /sys/kernel/debug/tracing/trace
>   ...
>   sh-422   [000] d... 650.495930: sys_write: (SyS_write+0x0/0xc8) count=22
>   sh-422   [000] d... 651.102389: sys_write: (SyS_write+0x0/0xc8) count=26
>   sh-422   [000] d... 651.358653: sys_write: (SyS_write+0x0/0xc8) count=86

Ah, good catch!

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/include/dwarf-regs-table.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> index 2675936..36e375f 100644
> --- a/tools/perf/arch/arm64/include/dwarf-regs-table.h
> +++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> @@ -2,12 +2,12 @@
>  /* This is included in perf/util/dwarf-regs.c */
>  
>  static const char * const aarch64_regstr_tbl[] = {
> -	"%r0", "%r1", "%r2", "%r3", "%r4",
> -	"%r5", "%r6", "%r7", "%r8", "%r9",
> -	"%r10", "%r11", "%r12", "%r13", "%r14",
> -	"%r15", "%r16", "%r17", "%r18", "%r19",
> -	"%r20", "%r21", "%r22", "%r23", "%r24",
> -	"%r25", "%r26", "%r27", "%r28", "%r29",
> +	"%x0", "%x1", "%x2", "%x3", "%x4",
> +	"%x5", "%x6", "%x7", "%x8", "%x9",
> +	"%x10", "%x11", "%x12", "%x13", "%x14",
> +	"%x15", "%x16", "%x17", "%x18", "%x19",
> +	"%x20", "%x21", "%x22", "%x23", "%x24",
> +	"%x25", "%x26", "%x27", "%x28", "%x29",
>  	"%lr", "%sp",
>  };
>  #endif
> -- 
> 1.8.5.2
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Masami Hiramatsu]

On Wed, 25 Jan 2017 13:32:01 +0000
Will Deacon <will.deacon@arm.com> wrote:

> On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > regs_query_register_offset() to convert register name to offset for
> > arm64, so the BPF prologue feature is ready to use.
> > 
> > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > so the related functions are consistent with x86.
> 
> Wouldn't it be an awful lot simpler just to leave the code as-is, and
> implement regs_query_register_offset in the same way that we implement
> get_arch_regstr but return the dwarfnum?

No, since the offset is not same as dwarfnum.

With this style, the index of array becomes the dwarfnum (the index of
each register defined by DWARF) and the "offset" member means the
byte-offset of the register in (user_)pt_regs. Those should be different.

> I don't really see the point of all the refactoring.

Also, from the maintenance point of view, this rewrite work makes
the code simply similar to x86 implementation, that will be easier to
maintain :)

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Masami Hiramatsu]

On Wed, 25 Jan 2017 13:32:01 +0000
Will Deacon <will.deacon@arm.com> wrote:

> On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > regs_query_register_offset() to convert register name to offset for
> > arm64, so the BPF prologue feature is ready to use.
> > 
> > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > so the related functions are consistent with x86.
> 
> Wouldn't it be an awful lot simpler just to leave the code as-is, and
> implement regs_query_register_offset in the same way that we implement
> get_arch_regstr but return the dwarfnum?

No, since the offset is not same as dwarfnum.

With this style, the index of array becomes the dwarfnum (the index of
each register defined by DWARF) and the "offset" member means the
byte-offset of the register in (user_)pt_regs. Those should be different.

> I don't really see the point of all the refactoring.

Also, from the maintenance point of view, this rewrite work makes
the code simply similar to x86 implementation, that will be easier to
maintain :)

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Masami Hiramatsu]

On Wed, 25 Jan 2017 07:23:11 +0000
He Kuang <hekuang@huawei.com> wrote:

> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> regs_query_register_offset() to convert register name to offset for
> arm64, so the BPF prologue feature is ready to use.
> 
> This patch also changes the 'dwarfnum' to 'offset' in register table,
> so the related functions are consistent with x86.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/Makefile          |   1 +
>  tools/perf/arch/arm64/util/dwarf-regs.c | 124 ++++++++++++++++++--------------
>  2 files changed, 72 insertions(+), 53 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
> index 18b1351..eebe1ec 100644
> --- a/tools/perf/arch/arm64/Makefile
> +++ b/tools/perf/arch/arm64/Makefile
> @@ -2,3 +2,4 @@ ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
>  PERF_HAVE_JITDUMP := 1
> +PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
> diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
> index d49efeb..5ec62a4 100644
> --- a/tools/perf/arch/arm64/util/dwarf-regs.c
> +++ b/tools/perf/arch/arm64/util/dwarf-regs.c
> @@ -9,72 +9,90 @@
>   */
>  
>  #include <stddef.h>
> +#include <linux/ptrace.h> /* for struct user_pt_regs */
> +#include <errno.h> /* for EINVAL */
> +#include <string.h> /* for strcmp */
> +#include <linux/ptrace.h> /* for struct user_pt_regs */

Here is a duplicated line.

Other parts look good to me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
except for the above line.

Thank you!

>  #include <dwarf-regs.h>
>  
> -struct pt_regs_dwarfnum {
> +struct pt_regs_offset {
>  	const char *name;
> -	unsigned int dwarfnum;
> +	int offset;
>  };
>  
> -#define STR(s) #s
> -#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
> -#define GPR_DWARFNUM_NAME(num) \
> -	{.name = STR(%x##num), .dwarfnum = num}
> -#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
> -
>  /*
>   * Reference:
>   * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
>   */
> -static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
> -	GPR_DWARFNUM_NAME(0),
> -	GPR_DWARFNUM_NAME(1),
> -	GPR_DWARFNUM_NAME(2),
> -	GPR_DWARFNUM_NAME(3),
> -	GPR_DWARFNUM_NAME(4),
> -	GPR_DWARFNUM_NAME(5),
> -	GPR_DWARFNUM_NAME(6),
> -	GPR_DWARFNUM_NAME(7),
> -	GPR_DWARFNUM_NAME(8),
> -	GPR_DWARFNUM_NAME(9),
> -	GPR_DWARFNUM_NAME(10),
> -	GPR_DWARFNUM_NAME(11),
> -	GPR_DWARFNUM_NAME(12),
> -	GPR_DWARFNUM_NAME(13),
> -	GPR_DWARFNUM_NAME(14),
> -	GPR_DWARFNUM_NAME(15),
> -	GPR_DWARFNUM_NAME(16),
> -	GPR_DWARFNUM_NAME(17),
> -	GPR_DWARFNUM_NAME(18),
> -	GPR_DWARFNUM_NAME(19),
> -	GPR_DWARFNUM_NAME(20),
> -	GPR_DWARFNUM_NAME(21),
> -	GPR_DWARFNUM_NAME(22),
> -	GPR_DWARFNUM_NAME(23),
> -	GPR_DWARFNUM_NAME(24),
> -	GPR_DWARFNUM_NAME(25),
> -	GPR_DWARFNUM_NAME(26),
> -	GPR_DWARFNUM_NAME(27),
> -	GPR_DWARFNUM_NAME(28),
> -	GPR_DWARFNUM_NAME(29),
> -	REG_DWARFNUM_NAME("%lr", 30),
> -	REG_DWARFNUM_NAME("%sp", 31),
> -	REG_DWARFNUM_END,
> +#define REG_OFFSET_NAME(r, num) {.name = "%" #r,			\
> +			.offset = offsetof(struct user_pt_regs, regs[num])}
> +#define REG_OFFSET_END {.name = NULL, .offset = 0}
> +#define GPR_OFFSET_NAME(r) \
> +	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
> +
> +/* This table is for reverse searching for the offset or register
> + * names in aarch64_regstr_tbl[].
> + */
> +static const struct pt_regs_offset regoffset_table[] = {
> +	GPR_OFFSET_NAME(0),
> +	GPR_OFFSET_NAME(1),
> +	GPR_OFFSET_NAME(2),
> +	GPR_OFFSET_NAME(3),
> +	GPR_OFFSET_NAME(4),
> +	GPR_OFFSET_NAME(5),
> +	GPR_OFFSET_NAME(6),
> +	GPR_OFFSET_NAME(7),
> +	GPR_OFFSET_NAME(8),
> +	GPR_OFFSET_NAME(9),
> +	GPR_OFFSET_NAME(10),
> +	GPR_OFFSET_NAME(11),
> +	GPR_OFFSET_NAME(12),
> +	GPR_OFFSET_NAME(13),
> +	GPR_OFFSET_NAME(14),
> +	GPR_OFFSET_NAME(15),
> +	GPR_OFFSET_NAME(16),
> +	GPR_OFFSET_NAME(17),
> +	GPR_OFFSET_NAME(18),
> +	GPR_OFFSET_NAME(19),
> +	GPR_OFFSET_NAME(20),
> +	GPR_OFFSET_NAME(21),
> +	GPR_OFFSET_NAME(22),
> +	GPR_OFFSET_NAME(23),
> +	GPR_OFFSET_NAME(24),
> +	GPR_OFFSET_NAME(25),
> +	GPR_OFFSET_NAME(26),
> +	GPR_OFFSET_NAME(27),
> +	GPR_OFFSET_NAME(28),
> +	GPR_OFFSET_NAME(29),
> +	REG_OFFSET_NAME(lr, 30),
> +	REG_OFFSET_NAME(sp, 31),
> +	REG_OFFSET_END,
>  };
>  
> +/* Minus 1 for the ending REG_OFFSET_END */
> +#define ARCH_MAX_REGS ((sizeof(regoffset_table) /		\
> +			sizeof(regoffset_table[0])) - 1)
> +
> +/* Return architecture dependent register string (for kprobe-tracer) */
> +const char *get_arch_regstr(unsigned int n)
> +{
> +	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
> +}
> +
> +/* Reuse code from arch/arm64/kernel/ptrace.c */
>  /**
> - * get_arch_regstr() - lookup register name from it's DWARF register number
> - * @n:	the DWARF register number
> + * regs_query_register_offset() - query register offset from its name
> + * @name:	the name of a register
>   *
> - * get_arch_regstr() returns the name of the register in struct
> - * regdwarfnum_table from it's DWARF register number. If the register is not
> - * found in the table, this returns NULL;
> + * regs_query_register_offset() returns the offset of a register in struct
> + * user_pt_regs from its name. If the name is invalid, this returns -EINVAL;
>   */
> -const char *get_arch_regstr(unsigned int n)
> +int regs_query_register_offset(const char *name)
>  {
> -	const struct pt_regs_dwarfnum *roff;
> -	for (roff = regdwarfnum_table; roff->name != NULL; roff++)
> -		if (roff->dwarfnum == n)
> -			return roff->name;
> -	return NULL;
> +	const struct pt_regs_offset *roff;
> +
> +	for (roff = regoffset_table; roff->name != NULL; roff++)
> +		if (!strcmp(roff->name, name))
> +			return roff->offset;
> +	return -EINVAL;
>  }
> -- 
> 1.8.5.2
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Masami Hiramatsu]

On Wed, 25 Jan 2017 07:23:11 +0000
He Kuang <hekuang@huawei.com> wrote:

> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> regs_query_register_offset() to convert register name to offset for
> arm64, so the BPF prologue feature is ready to use.
> 
> This patch also changes the 'dwarfnum' to 'offset' in register table,
> so the related functions are consistent with x86.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/Makefile          |   1 +
>  tools/perf/arch/arm64/util/dwarf-regs.c | 124 ++++++++++++++++++--------------
>  2 files changed, 72 insertions(+), 53 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
> index 18b1351..eebe1ec 100644
> --- a/tools/perf/arch/arm64/Makefile
> +++ b/tools/perf/arch/arm64/Makefile
> @@ -2,3 +2,4 @@ ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
>  PERF_HAVE_JITDUMP := 1
> +PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
> diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c
> index d49efeb..5ec62a4 100644
> --- a/tools/perf/arch/arm64/util/dwarf-regs.c
> +++ b/tools/perf/arch/arm64/util/dwarf-regs.c
> @@ -9,72 +9,90 @@
>   */
>  
>  #include <stddef.h>
> +#include <linux/ptrace.h> /* for struct user_pt_regs */
> +#include <errno.h> /* for EINVAL */
> +#include <string.h> /* for strcmp */
> +#include <linux/ptrace.h> /* for struct user_pt_regs */

Here is a duplicated line.

Other parts look good to me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
except for the above line.

Thank you!

>  #include <dwarf-regs.h>
>  
> -struct pt_regs_dwarfnum {
> +struct pt_regs_offset {
>  	const char *name;
> -	unsigned int dwarfnum;
> +	int offset;
>  };
>  
> -#define STR(s) #s
> -#define REG_DWARFNUM_NAME(r, num) {.name = r, .dwarfnum = num}
> -#define GPR_DWARFNUM_NAME(num) \
> -	{.name = STR(%x##num), .dwarfnum = num}
> -#define REG_DWARFNUM_END {.name = NULL, .dwarfnum = 0}
> -
>  /*
>   * Reference:
>   * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
>   */
> -static const struct pt_regs_dwarfnum regdwarfnum_table[] = {
> -	GPR_DWARFNUM_NAME(0),
> -	GPR_DWARFNUM_NAME(1),
> -	GPR_DWARFNUM_NAME(2),
> -	GPR_DWARFNUM_NAME(3),
> -	GPR_DWARFNUM_NAME(4),
> -	GPR_DWARFNUM_NAME(5),
> -	GPR_DWARFNUM_NAME(6),
> -	GPR_DWARFNUM_NAME(7),
> -	GPR_DWARFNUM_NAME(8),
> -	GPR_DWARFNUM_NAME(9),
> -	GPR_DWARFNUM_NAME(10),
> -	GPR_DWARFNUM_NAME(11),
> -	GPR_DWARFNUM_NAME(12),
> -	GPR_DWARFNUM_NAME(13),
> -	GPR_DWARFNUM_NAME(14),
> -	GPR_DWARFNUM_NAME(15),
> -	GPR_DWARFNUM_NAME(16),
> -	GPR_DWARFNUM_NAME(17),
> -	GPR_DWARFNUM_NAME(18),
> -	GPR_DWARFNUM_NAME(19),
> -	GPR_DWARFNUM_NAME(20),
> -	GPR_DWARFNUM_NAME(21),
> -	GPR_DWARFNUM_NAME(22),
> -	GPR_DWARFNUM_NAME(23),
> -	GPR_DWARFNUM_NAME(24),
> -	GPR_DWARFNUM_NAME(25),
> -	GPR_DWARFNUM_NAME(26),
> -	GPR_DWARFNUM_NAME(27),
> -	GPR_DWARFNUM_NAME(28),
> -	GPR_DWARFNUM_NAME(29),
> -	REG_DWARFNUM_NAME("%lr", 30),
> -	REG_DWARFNUM_NAME("%sp", 31),
> -	REG_DWARFNUM_END,
> +#define REG_OFFSET_NAME(r, num) {.name = "%" #r,			\
> +			.offset = offsetof(struct user_pt_regs, regs[num])}
> +#define REG_OFFSET_END {.name = NULL, .offset = 0}
> +#define GPR_OFFSET_NAME(r) \
> +	{.name = "%x" #r, .offset = offsetof(struct user_pt_regs, regs[r])}
> +
> +/* This table is for reverse searching for the offset or register
> + * names in aarch64_regstr_tbl[].
> + */
> +static const struct pt_regs_offset regoffset_table[] = {
> +	GPR_OFFSET_NAME(0),
> +	GPR_OFFSET_NAME(1),
> +	GPR_OFFSET_NAME(2),
> +	GPR_OFFSET_NAME(3),
> +	GPR_OFFSET_NAME(4),
> +	GPR_OFFSET_NAME(5),
> +	GPR_OFFSET_NAME(6),
> +	GPR_OFFSET_NAME(7),
> +	GPR_OFFSET_NAME(8),
> +	GPR_OFFSET_NAME(9),
> +	GPR_OFFSET_NAME(10),
> +	GPR_OFFSET_NAME(11),
> +	GPR_OFFSET_NAME(12),
> +	GPR_OFFSET_NAME(13),
> +	GPR_OFFSET_NAME(14),
> +	GPR_OFFSET_NAME(15),
> +	GPR_OFFSET_NAME(16),
> +	GPR_OFFSET_NAME(17),
> +	GPR_OFFSET_NAME(18),
> +	GPR_OFFSET_NAME(19),
> +	GPR_OFFSET_NAME(20),
> +	GPR_OFFSET_NAME(21),
> +	GPR_OFFSET_NAME(22),
> +	GPR_OFFSET_NAME(23),
> +	GPR_OFFSET_NAME(24),
> +	GPR_OFFSET_NAME(25),
> +	GPR_OFFSET_NAME(26),
> +	GPR_OFFSET_NAME(27),
> +	GPR_OFFSET_NAME(28),
> +	GPR_OFFSET_NAME(29),
> +	REG_OFFSET_NAME(lr, 30),
> +	REG_OFFSET_NAME(sp, 31),
> +	REG_OFFSET_END,
>  };
>  
> +/* Minus 1 for the ending REG_OFFSET_END */
> +#define ARCH_MAX_REGS ((sizeof(regoffset_table) /		\
> +			sizeof(regoffset_table[0])) - 1)
> +
> +/* Return architecture dependent register string (for kprobe-tracer) */
> +const char *get_arch_regstr(unsigned int n)
> +{
> +	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
> +}
> +
> +/* Reuse code from arch/arm64/kernel/ptrace.c */
>  /**
> - * get_arch_regstr() - lookup register name from it's DWARF register number
> - * @n:	the DWARF register number
> + * regs_query_register_offset() - query register offset from its name
> + * @name:	the name of a register
>   *
> - * get_arch_regstr() returns the name of the register in struct
> - * regdwarfnum_table from it's DWARF register number. If the register is not
> - * found in the table, this returns NULL;
> + * regs_query_register_offset() returns the offset of a register in struct
> + * user_pt_regs from its name. If the name is invalid, this returns -EINVAL;
>   */
> -const char *get_arch_regstr(unsigned int n)
> +int regs_query_register_offset(const char *name)
>  {
> -	const struct pt_regs_dwarfnum *roff;
> -	for (roff = regdwarfnum_table; roff->name != NULL; roff++)
> -		if (roff->dwarfnum == n)
> -			return roff->name;
> -	return NULL;
> +	const struct pt_regs_offset *roff;
> +
> +	for (roff = regoffset_table; roff->name != NULL; roff++)
> +		if (!strcmp(roff->name, name))
> +			return roff->offset;
> +	return -EINVAL;
>  }
> -- 
> 1.8.5.2
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[tip:perf/core] perf probe: Fix wrong register name for arm64

[tip-bot for He Kuang]

Commit-ID:  1b29dfbba124be5077a24996a272205baec1c008
Gitweb:     http://git.kernel.org/tip/1b29dfbba124be5077a24996a272205baec1c008
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Tue, 24 Jan 2017 10:30:14 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 26 Jan 2017 11:42:43 -0300

perf probe: Fix wrong register name for arm64

The register name of arm64 architecture is x0-x31 not r0-r31, this patch
changes this typo.

Before this patch:

  # perf probe --definition 'sys_write count'
  p:probe/sys_write _text+1502872 count=%r2:s64

  # echo 'p:probe/sys_write _text+1502872 count=%r2:s64' > \
    /sys/kernel/debug/tracing/kprobe_events
  Parse error at argument[0]. (-22)

After this patch:

  # perf probe --definition 'sys_write count'
  p:probe/sys_write _text+1502872 count=%x2:s64

  # echo 'p:probe/sys_write _text+1502872 count=%x2:s64' > \
    /sys/kernel/debug/tracing/kprobe_events
  # echo 1 >/sys/kernel/debug/tracing/events/probe/enable
  # cat /sys/kernel/debug/tracing/trace
  ...
  sh-422   [000] d... 650.495930: sys_write: (SyS_write+0x0/0xc8) count=22
  sh-422   [000] d... 651.102389: sys_write: (SyS_write+0x0/0xc8) count=26
  sh-422   [000] d... 651.358653: sys_write: (SyS_write+0x0/0xc8) count=86

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Bintian Wang <bintian.wang@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170124103015.1936-2-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/arm64/include/dwarf-regs-table.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
index 2675936..36e375f 100644
--- a/tools/perf/arch/arm64/include/dwarf-regs-table.h
+++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
@@ -2,12 +2,12 @@
 /* This is included in perf/util/dwarf-regs.c */
 
 static const char * const aarch64_regstr_tbl[] = {
-	"%r0", "%r1", "%r2", "%r3", "%r4",
-	"%r5", "%r6", "%r7", "%r8", "%r9",
-	"%r10", "%r11", "%r12", "%r13", "%r14",
-	"%r15", "%r16", "%r17", "%r18", "%r19",
-	"%r20", "%r21", "%r22", "%r23", "%r24",
-	"%r25", "%r26", "%r27", "%r28", "%r29",
+	"%x0", "%x1", "%x2", "%x3", "%x4",
+	"%x5", "%x6", "%x7", "%x8", "%x9",
+	"%x10", "%x11", "%x12", "%x13", "%x14",
+	"%x15", "%x16", "%x17", "%x18", "%x19",
+	"%x20", "%x21", "%x22", "%x23", "%x24",
+	"%x25", "%x26", "%x27", "%x28", "%x29",
 	"%lr", "%sp",
 };
 #endif

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Will Deacon]

On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
> On Wed, 25 Jan 2017 13:32:01 +0000
> Will Deacon <will.deacon@arm.com> wrote:
> 
> > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > > regs_query_register_offset() to convert register name to offset for
> > > arm64, so the BPF prologue feature is ready to use.
> > > 
> > > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > > so the related functions are consistent with x86.
> > 
> > Wouldn't it be an awful lot simpler just to leave the code as-is, and
> > implement regs_query_register_offset in the same way that we implement
> > get_arch_regstr but return the dwarfnum?
> 
> No, since the offset is not same as dwarfnum.
> 
> With this style, the index of array becomes the dwarfnum (the index of
> each register defined by DWARF) and the "offset" member means the
> byte-offset of the register in (user_)pt_regs. Those should be different.

Ok, then do it as two patches then, rather than introduce functionality
along with the renaming.

> > I don't really see the point of all the refactoring.
> 
> Also, from the maintenance point of view, this rewrite work makes
> the code simply similar to x86 implementation, that will be easier to
> maintain :)

Right, apart from the two howling bugs in the version that was nearly merged
initially :p. I tend to err on the "if it ain't broke, don't fix it" side
of the argument but if you really want the refactoring lets keep it as a
separate change.

Will

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Will Deacon]

On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
> On Wed, 25 Jan 2017 13:32:01 +0000
> Will Deacon <will.deacon@arm.com> wrote:
> 
> > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > > regs_query_register_offset() to convert register name to offset for
> > > arm64, so the BPF prologue feature is ready to use.
> > > 
> > > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > > so the related functions are consistent with x86.
> > 
> > Wouldn't it be an awful lot simpler just to leave the code as-is, and
> > implement regs_query_register_offset in the same way that we implement
> > get_arch_regstr but return the dwarfnum?
> 
> No, since the offset is not same as dwarfnum.
> 
> With this style, the index of array becomes the dwarfnum (the index of
> each register defined by DWARF) and the "offset" member means the
> byte-offset of the register in (user_)pt_regs. Those should be different.

Ok, then do it as two patches then, rather than introduce functionality
along with the renaming.

> > I don't really see the point of all the refactoring.
> 
> Also, from the maintenance point of view, this rewrite work makes
> the code simply similar to x86 implementation, that will be easier to
> maintain :)

Right, apart from the two howling bugs in the version that was nearly merged
initially :p. I tend to err on the "if it ain't broke, don't fix it" side
of the argument but if you really want the refactoring lets keep it as a
separate change.

Will

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Arnaldo Carvalho de Melo]

Em Thu, Jan 26, 2017 at 04:52:12PM +0000, Will Deacon escreveu:
> On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
> > On Wed, 25 Jan 2017 13:32:01 +0000
> > Will Deacon <will.deacon@arm.com> wrote:
> > 
> > > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > > > regs_query_register_offset() to convert register name to offset for
> > > > arm64, so the BPF prologue feature is ready to use.
> > > > 
> > > > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > > > so the related functions are consistent with x86.
> > > 
> > > Wouldn't it be an awful lot simpler just to leave the code as-is, and
> > > implement regs_query_register_offset in the same way that we implement
> > > get_arch_regstr but return the dwarfnum?
> > 
> > No, since the offset is not same as dwarfnum.
> > 
> > With this style, the index of array becomes the dwarfnum (the index of
> > each register defined by DWARF) and the "offset" member means the
> > byte-offset of the register in (user_)pt_regs. Those should be different.
> 
> Ok, then do it as two patches then, rather than introduce functionality
> along with the renaming.
> 
> > > I don't really see the point of all the refactoring.
> > 
> > Also, from the maintenance point of view, this rewrite work makes
> > the code simply similar to x86 implementation, that will be easier to
> > maintain :)
> 
> Right, apart from the two howling bugs in the version that was nearly merged
> initially :p. I tend to err on the "if it ain't broke, don't fix it" side
> of the argument but if you really want the refactoring lets keep it as a
> separate change.

So, He, can you do that? How do we proceed?

- Arnaldo

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Arnaldo Carvalho de Melo]

Em Thu, Jan 26, 2017 at 04:52:12PM +0000, Will Deacon escreveu:
> On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
> > On Wed, 25 Jan 2017 13:32:01 +0000
> > Will Deacon <will.deacon@arm.com> wrote:
> > 
> > > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
> > > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces
> > > > regs_query_register_offset() to convert register name to offset for
> > > > arm64, so the BPF prologue feature is ready to use.
> > > > 
> > > > This patch also changes the 'dwarfnum' to 'offset' in register table,
> > > > so the related functions are consistent with x86.
> > > 
> > > Wouldn't it be an awful lot simpler just to leave the code as-is, and
> > > implement regs_query_register_offset in the same way that we implement
> > > get_arch_regstr but return the dwarfnum?
> > 
> > No, since the offset is not same as dwarfnum.
> > 
> > With this style, the index of array becomes the dwarfnum (the index of
> > each register defined by DWARF) and the "offset" member means the
> > byte-offset of the register in (user_)pt_regs. Those should be different.
> 
> Ok, then do it as two patches then, rather than introduce functionality
> along with the renaming.
> 
> > > I don't really see the point of all the refactoring.
> > 
> > Also, from the maintenance point of view, this rewrite work makes
> > the code simply similar to x86 implementation, that will be easier to
> > maintain :)
> 
> Right, apart from the two howling bugs in the version that was nearly merged
> initially :p. I tend to err on the "if it ain't broke, don't fix it" side
> of the argument but if you really want the refactoring lets keep it as a
> separate change.

So, He, can you do that? How do we proceed?

- Arnaldo

Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Hekuang]

hi,

在 2017/1/27 3:31, Arnaldo Carvalho de Melo 写道:
> Em Thu, Jan 26, 2017 at 04:52:12PM +0000, Will Deacon escreveu:
>> On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
>>> On Wed, 25 Jan 2017 13:32:01 +0000
>>> Will Deacon <will.deacon@arm.com> wrote:
>>>
>>>> On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
>>>>> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
>>>>> regs_query_register_offset() to convert register name to offset for
>>>>> arm64, so the BPF prologue feature is ready to use.
>>>>>
>>>>> This patch also changes the 'dwarfnum' to 'offset' in register table,
>>>>> so the related functions are consistent with x86.
>>>> Wouldn't it be an awful lot simpler just to leave the code as-is, and
>>>> implement regs_query_register_offset in the same way that we implement
>>>> get_arch_regstr but return the dwarfnum?
>>> No, since the offset is not same as dwarfnum.
>>>
>>> With this style, the index of array becomes the dwarfnum (the index of
>>> each register defined by DWARF) and the "offset" member means the
>>> byte-offset of the register in (user_)pt_regs. Those should be different.
>> Ok, then do it as two patches then, rather than introduce functionality
>> along with the renaming.
>>
>>>> I don't really see the point of all the refactoring.
>>> Also, from the maintenance point of view, this rewrite work makes
>>> the code simply similar to x86 implementation, that will be easier to
>>> maintain :)
>> Right, apart from the two howling bugs in the version that was nearly merged
>> initially :p. I tend to err on the "if it ain't broke, don't fix it" side
>> of the argument but if you really want the refactoring lets keep it as a
>> separate change.
> So, He, can you do that? How do we proceed?
>
> - Arnaldo

I split the patch as Will suggested and resend them. Sorry for
late response, just back from Spring festival.

[PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64

[Hekuang]

hi,

? 2017/1/27 3:31, Arnaldo Carvalho de Melo ??:
> Em Thu, Jan 26, 2017 at 04:52:12PM +0000, Will Deacon escreveu:
>> On Thu, Jan 26, 2017 at 10:49:16AM +0900, Masami Hiramatsu wrote:
>>> On Wed, 25 Jan 2017 13:32:01 +0000
>>> Will Deacon <will.deacon@arm.com> wrote:
>>>
>>>> On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote:
>>>>> Since HAVE_KPROBES can be enabled in arm64, this patch introduces
>>>>> regs_query_register_offset() to convert register name to offset for
>>>>> arm64, so the BPF prologue feature is ready to use.
>>>>>
>>>>> This patch also changes the 'dwarfnum' to 'offset' in register table,
>>>>> so the related functions are consistent with x86.
>>>> Wouldn't it be an awful lot simpler just to leave the code as-is, and
>>>> implement regs_query_register_offset in the same way that we implement
>>>> get_arch_regstr but return the dwarfnum?
>>> No, since the offset is not same as dwarfnum.
>>>
>>> With this style, the index of array becomes the dwarfnum (the index of
>>> each register defined by DWARF) and the "offset" member means the
>>> byte-offset of the register in (user_)pt_regs. Those should be different.
>> Ok, then do it as two patches then, rather than introduce functionality
>> along with the renaming.
>>
>>>> I don't really see the point of all the refactoring.
>>> Also, from the maintenance point of view, this rewrite work makes
>>> the code simply similar to x86 implementation, that will be easier to
>>> maintain :)
>> Right, apart from the two howling bugs in the version that was nearly merged
>> initially :p. I tend to err on the "if it ain't broke, don't fix it" side
>> of the argument but if you really want the refactoring lets keep it as a
>> separate change.
> So, He, can you do that? How do we proceed?
>
> - Arnaldo

I split the patch as Will suggested and resend them. Sorry for
late response, just back from Spring festival.

Re: [PATCH 1/2] perf probe: Fix wrong register name for arm64

[Pratyush Anand]

Just noticed that this patch fixes commit e47392bf9c06 (v4.8+). So,
Ccing stable@vger.kernel.org
[e47392bf9c06 perf uprobe: Skip prologue if program compiled without
optimization]

After e47392bf9c06 on ARM64:
# cat > test.c << EOF
> #include <stdio.h>
>
> int main(int argc, void **argv)
> {
>         printf("argc =%d\n", argc);
> }
> EOF
# gcc -g  -o test test.c
# perf probe -x ./test -a 'main argc'
Target program is compiled without optimization. Skipping prologue.
Probe on address 0x400600 to force probing at the function entry.

Failed to write event: Invalid argument
  Error: Failed to add events.

If this patch is applied on top of e47392bf9c06:
perf probe -x ./test -a 'main argc'
Target program is compiled without optimization. Skipping prologue.
Probe on address 0x400600 to force probing at the function entry.

Added new event:
  probe_test:main      (on main in
/home/panand/work/test_code/uprobe_test/test with argc)

You can now use it in all perf tools, such as:

        perf record -e probe_test:main -aR sleep 1

~Pratyush

On Tue, Jan 24, 2017 at 4:00 PM, He Kuang <hekuang@huawei.com> wrote:
> The register name of arm64 architecture is x0-x31 not r0-r31, this
> patch changes this typo.
>
> Before this patch:
>
>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%r2:s64
>
>   # echo 'p:probe/sys_write _text+1502872 count=%r2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   Parse error at argument[0]. (-22)
>
> After this patch:
>
>   # perf probe --definition 'sys_write count'
>   p:probe/sys_write _text+1502872 count=%x2:s64
>
>   # echo 'p:probe/sys_write _text+1502872 count=%x2:s64' > \
>     /sys/kernel/debug/tracing/kprobe_events
>   # echo 1 >/sys/kernel/debug/tracing/events/probe/enable
>   # cat /sys/kernel/debug/tracing/trace
>   ...
>   sh-422   [000] d... 650.495930: sys_write: (SyS_write+0x0/0xc8) count=22
>   sh-422   [000] d... 651.102389: sys_write: (SyS_write+0x0/0xc8) count=26
>   sh-422   [000] d... 651.358653: sys_write: (SyS_write+0x0/0xc8) count=86
>
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/arch/arm64/include/dwarf-regs-table.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/arch/arm64/include/dwarf-regs-table.h b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> index 2675936..36e375f 100644
> --- a/tools/perf/arch/arm64/include/dwarf-regs-table.h
> +++ b/tools/perf/arch/arm64/include/dwarf-regs-table.h
> @@ -2,12 +2,12 @@
>  /* This is included in perf/util/dwarf-regs.c */
>
>  static const char * const aarch64_regstr_tbl[] = {
> -       "%r0", "%r1", "%r2", "%r3", "%r4",
> -       "%r5", "%r6", "%r7", "%r8", "%r9",
> -       "%r10", "%r11", "%r12", "%r13", "%r14",
> -       "%r15", "%r16", "%r17", "%r18", "%r19",
> -       "%r20", "%r21", "%r22", "%r23", "%r24",
> -       "%r25", "%r26", "%r27", "%r28", "%r29",
> +       "%x0", "%x1", "%x2", "%x3", "%x4",
> +       "%x5", "%x6", "%x7", "%x8", "%x9",
> +       "%x10", "%x11", "%x12", "%x13", "%x14",
> +       "%x15", "%x16", "%x17", "%x18", "%x19",
> +       "%x20", "%x21", "%x22", "%x23", "%x24",
> +       "%x25", "%x26", "%x27", "%x28", "%x29",
>         "%lr", "%sp",
>  };
>  #endif
> --
> 1.8.5.2
>