| Message ID | 1440822125-52691-24-git-send-email-wangnan0@huawei.com |
|---|---|
| State | New |
| Headers | show |
Em Sat, Aug 29, 2015 at 04:21:57AM +0000, Wang Nan escreveu: > From: He Kuang <hekuang@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? 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 <wangnan0@huawei.com> > Signed-off-by: He Kuang <hekuang@huawei.com> > Cc: Alexei Starovoitov <ast@plumgrid.com> > Cc: Brendan Gregg <brendan.d.gregg@gmail.com> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: David Ahern <dsahern@gmail.com> > Cc: He Kuang <hekuang@huawei.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Kaixu Xia <xiakaixu@huawei.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Zefan Li <lizefan@huawei.com> > Cc: pi3orama@163.com > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Link: http://lkml.kernel.org/n/1436445342-1402-34-git-send-email-wangnan0@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <hekuang@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 <wangnan0@huawei.com> >> Signed-off-by: He Kuang <hekuang@huawei.com> >> Cc: Alexei Starovoitov <ast@plumgrid.com> >> Cc: Brendan Gregg <brendan.d.gregg@gmail.com> >> Cc: Daniel Borkmann <daniel@iogearbox.net> >> Cc: David Ahern <dsahern@gmail.com> >> Cc: He Kuang <hekuang@huawei.com> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Cc: Kaixu Xia <xiakaixu@huawei.com> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >> Cc: Zefan Li <lizefan@huawei.com> >> Cc: pi3orama@163.com >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> Link: http://lkml.kernel.org/n/1436445342-1402-34-git-send-email-wangnan0@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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