On 2015/9/1 4:43, Arnaldo Carvalho de Melo wrote:
Em Sat, Aug 29, 2015 at 04:21:57AM +0000, Wang Nan escreveu:
From: He Kuang <heku...@huawei.com>

arch_get_reg_info() is a helper function which converts register name
like "%rax" to offset of a register in 'struct pt_regs', which is
required by BPF prologue generator.
Is this something like:

/* Query offset/name of register from its name/offset */
extern int regs_query_register_offset(const char *name);

in ptrace? Can't we reuse that name and even code?

Unfortunately we can't reuse its code, because pt_regs is defined differently
in user and kernel side.

In arch/x86/kernel/ptrace.c we have:

struct pt_regs_offset {
        const char *name;
        int offset;
};

#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct pt_regs, r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}

static const struct pt_regs_offset regoffset_table[] = {
#ifdef CONFIG_X86_64
...
    REG_OFFSET_NAME(r15),
    REG_OFFSET_NAME(r14),
    REG_OFFSET_NAME(r13),
...

The definition of REG_OFFSET_NAME relys on the field name and the string name of a
register are identical. This is true for kernel, but not true for userspace.

For example, for x86_64, 'struct pt_regs' is defined in arch/x86/include/asm/ptrace.h for kernel, and the reigster name is 'ax, cx, dx, si, di ...'. In contract, which is defined in arch/x86/include/uapi/asm/ptrace.h for user, and the register name becomes
'rax, rcx, rdx, rsi, rdi ...'.

Since logical of regs_query_register_offset() is very simple, changing REG_OFFSET_NAME()
makes it a totally different function.

But yes, let's reuse its name. And it may worth considering to reuse its code for other
archs.

Thank you.

Was this that was done and only a rename was made?

- Arnaldo

This patch replaces original string table by a 'struct reg_info' table,
which records offset of registers according to its name.

For x86, since there are two sub-archs (x86_32 and x86_64) but we can
only get pt_regs for the arch we are currently on, this patch fills
offset with '-1' for another sub-arch. This introduces a limitation to
perf prologue that, we are unable to generate prologue on a x86_32
compiled perf for BPF programs targeted on x86_64 kernel. This
limitation is acceptable, because this is a very rare usecase.

Signed-off-by: Wang Nan <wangn...@huawei.com>
Signed-off-by: He Kuang <heku...@huawei.com>
Cc: Alexei Starovoitov <a...@plumgrid.com>
Cc: Brendan Gregg <brendan.d.gr...@gmail.com>
Cc: Daniel Borkmann <dan...@iogearbox.net>
Cc: David Ahern <dsah...@gmail.com>
Cc: He Kuang <heku...@huawei.com>
Cc: Jiri Olsa <jo...@kernel.org>
Cc: Kaixu Xia <xiaka...@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
Cc: Namhyung Kim <namhy...@kernel.org>
Cc: Paul Mackerras <pau...@samba.org>
Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
Cc: Zefan Li <lize...@huawei.com>
Cc: pi3or...@163.com
Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
Link: 
http://lkml.kernel.org/n/1436445342-1402-34-git-send-email-wangn...@huawei.com
---
  tools/perf/arch/x86/Makefile          |   1 +
  tools/perf/arch/x86/util/Build        |   2 +
  tools/perf/arch/x86/util/dwarf-regs.c | 104 ++++++++++++++++++++++++----------
  3 files changed, 78 insertions(+), 29 deletions(-)

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 21322e0..a84a6f6f 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -2,3 +2,4 @@ ifndef NO_DWARF
  PERF_HAVE_DWARF_REGS := 1
  endif
  HAVE_KVM_STAT_SUPPORT := 1
+PERF_HAVE_ARCH_GET_REG_INFO := 1
diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
index 2c55e1b..09429f6 100644
--- a/tools/perf/arch/x86/util/Build
+++ b/tools/perf/arch/x86/util/Build
@@ -3,6 +3,8 @@ libperf-y += tsc.o
  libperf-y += pmu.o
  libperf-y += kvm-stat.o
+# BPF_PROLOGUE also need dwarf-regs.o. However, if CONFIG_BPF_PROLOGUE
+# is true, CONFIG_DWARF must true.
  libperf-$(CONFIG_DWARF) += dwarf-regs.o
libperf-$(CONFIG_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/x86/util/dwarf-regs.c 
b/tools/perf/arch/x86/util/dwarf-regs.c
index be22dd4..9928caf 100644
--- a/tools/perf/arch/x86/util/dwarf-regs.c
+++ b/tools/perf/arch/x86/util/dwarf-regs.c
@@ -22,44 +22,67 @@
#include <stddef.h>
  #include <dwarf-regs.h>
+#include <string.h>
+#include <linux/ptrace.h>
+#include <linux/kernel.h> /* for offsetof */
+#include <util/bpf-loader.h>
+
+struct reg_info {
+       const char      *name;          /* Reg string in debuginfo      */
+       int             offset;         /* Reg offset in struct pt_regs */
+};
/*
   * Generic dwarf analysis helpers
   */
-
+/*
+ * x86_64 compiling can't access pt_regs for x86_32, so fill offset
+ * with -1.
+ */
+#ifdef __x86_64__
+# define REG_INFO(n, f) { .name = n, .offset = -1, }
+#else
+# define REG_INFO(n, f) { .name = n, .offset = offsetof(struct pt_regs, f), }
+#endif
  #define X86_32_MAX_REGS 8
-const char *x86_32_regs_table[X86_32_MAX_REGS] = {
-       "%ax",
-       "%cx",
-       "%dx",
-       "%bx",
-       "$stack",     /* Stack address instead of %sp */
-       "%bp",
-       "%si",
-       "%di",
+
+struct reg_info x86_32_regs_table[X86_32_MAX_REGS] = {
+       REG_INFO("%ax", eax),
+       REG_INFO("%cx", ecx),
+       REG_INFO("%dx", edx),
+       REG_INFO("%bx", ebx),
+       REG_INFO("$stack", esp),      /* Stack address instead of %sp */
+       REG_INFO("%bp", ebp),
+       REG_INFO("%si", esi),
+       REG_INFO("%di", edi),
  };
+#undef REG_INFO
+#ifdef __x86_64__
+# define REG_INFO(n, f) { .name = n, .offset = offsetof(struct pt_regs, f), }
+#else
+# define REG_INFO(n, f) { .name = n, .offset = -1, }
+#endif
  #define X86_64_MAX_REGS 16
-const char *x86_64_regs_table[X86_64_MAX_REGS] = {
-       "%ax",
-       "%dx",
-       "%cx",
-       "%bx",
-       "%si",
-       "%di",
-       "%bp",
-       "%sp",
-       "%r8",
-       "%r9",
-       "%r10",
-       "%r11",
-       "%r12",
-       "%r13",
-       "%r14",
-       "%r15",
+struct reg_info x86_64_regs_table[X86_64_MAX_REGS] = {
+       REG_INFO("%ax",               rax),
+       REG_INFO("%dx",               rdx),
+       REG_INFO("%cx",               rcx),
+       REG_INFO("%bx",               rbx),
+       REG_INFO("%si",               rsi),
+       REG_INFO("%di",               rdi),
+       REG_INFO("%bp",               rbp),
+       REG_INFO("%sp",               rsp),
+       REG_INFO("%r8",               r8),
+       REG_INFO("%r9",               r9),
+       REG_INFO("%r10",      r10),
+       REG_INFO("%r11",      r11),
+       REG_INFO("%r12",      r12),
+       REG_INFO("%r13",      r13),
+       REG_INFO("%r14",      r14),
+       REG_INFO("%r15",      r15),
  };
-/* TODO: switching by dwarf address size */
  #ifdef __x86_64__
  #define ARCH_MAX_REGS X86_64_MAX_REGS
  #define arch_regs_table x86_64_regs_table
@@ -71,5 +94,28 @@ const char *x86_64_regs_table[X86_64_MAX_REGS] = {
  /* Return architecture dependent register string (for kprobe-tracer) */
  const char *get_arch_regstr(unsigned int n)
  {
-       return (n <= ARCH_MAX_REGS) ? arch_regs_table[n] : NULL;
+       return (n <= ARCH_MAX_REGS) ? arch_regs_table[n].name : NULL;
  }
+
+#ifdef HAVE_BPF_PROLOGUE
+int arch_get_reg_info(const char *name, int *offset)
+{
+       int i;
+       struct reg_info *info;
+
+       if (!name || !offset)
+               return -1;
+
+       for (i = 0; i < ARCH_MAX_REGS; i++) {
+               info = &arch_regs_table[i];
+               if (strcmp(info->name, name) == 0) {
+                       if (info->offset < 0)
+                               return -1;
+                       *offset = info->offset;
+                       return 0;
+               }
+       }
+
+       return -1;
+}
+#endif
--
2.1.0


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to