| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 1 Sep 2015 21:52:30 +0800
From: "Wangnan (F)" <wangnan0@...wei.com>
To: 平松雅巳 / HIRAMATU，MASAMI
<masami.hiramatsu.pt@...achi.com>,
"acme@...nel.org" <acme@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
He Kuang <hekuang@...wei.com>,
Alexei Starovoitov <ast@...mgrid.com>,
Brendan Gregg <brendan.d.gregg@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
"David Ahern" <dsahern@...il.com>, Jiri Olsa <jolsa@...nel.org>,
Kaixu Xia <xiakaixu@...wei.com>,
Namhyung Kim <namhyung@...nel.org>,
Paul Mackerras <paulus@...ba.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Zefan Li <lizefan@...wei.com>,
"pi3orama@....com" <pi3orama@....com>,
"Arnaldo Carvalho de Melo" <acme@...hat.com>
Subject: Re: [PATCH 23/31] perf tools: Introduce regs_query_register_offset()
for x86
On 2015/9/1 19:47, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: Wang Nan [mailto:wangnan0@...wei.com]
>>
>> regs_query_register_offset() 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. Since the function is
>> identical, try to reuse the code in arch/x86/kernel/ptrace.c.
>>
>> Comment inside dwarf-regs.c list the differences between this
>> implementation and kernel code.
> Hmm, this also introduce a duplication of the code...
> It might be a good time to move them into arch/x86/lib/ and
> reuse it directly from perf code.
So you want to move it from ./arch/x86/kernel/ptrace.c to arch/x86/lib and
let perf link against arch/x86/lib/lib.a to use it?
I think it worth a specific work to do it. Currently we lake
scaffold to compile and link against the kernel side library. Moreover,
we should also consider other archs. Seems not very easy.
This is not the only one file which can benifite from kernel's arch/x86/lib
Newly introduced tools/perf/util/intel-pt-decoder/insn.c, and I believe
there's
more. Therefore I think it should be a separated work from perf BPF patches.
So how about keep this patch at this time? Or do you have some idea?
Thank you.
> Thank you,
>
>> get_arch_regstr() switches to regoffset_table and the old string table
>> is dropped.
>>
>> Signed-off-by: Wang Nan <wangnan0@...wei.com>
>> Signed-off-by: He Kuang <hekuang@...wei.com>
>> Cc: Alexei Starovoitov <ast@...mgrid.com>
>> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
>> Cc: Daniel Borkmann <daniel@...earbox.net>
>> Cc: David Ahern <dsahern@...il.com>
>> Cc: He Kuang <hekuang@...wei.com>
>> Cc: Jiri Olsa <jolsa@...nel.org>
>> Cc: Kaixu Xia <xiakaixu@...wei.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>> Cc: Namhyung Kim <namhyung@...nel.org>
>> Cc: Paul Mackerras <paulus@...ba.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
>> Cc: Zefan Li <lizefan@...wei.com>
>> Cc: pi3orama@....com
>> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
>> ---
>> tools/perf/arch/x86/Makefile | 1 +
>> tools/perf/arch/x86/util/Build | 1 +
>> tools/perf/arch/x86/util/dwarf-regs.c | 122 ++++++++++++++++++++++++----------
>> 3 files changed, 90 insertions(+), 34 deletions(-)
>>
>> diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
>> index 21322e0..09ba923 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_REGS_QUERY_REGISTER_OFFSET := 1
>> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
>> index 2c55e1b..d4d1f23 100644
>> --- a/tools/perf/arch/x86/util/Build
>> +++ b/tools/perf/arch/x86/util/Build
>> @@ -4,6 +4,7 @@ libperf-y += pmu.o
>> libperf-y += kvm-stat.o
>>
>> libperf-$(CONFIG_DWARF) += dwarf-regs.o
>> +libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
>>
>> libperf-$(CONFIG_LIBUNWIND) += unwind-libunwind.o
>> libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
>> diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
>> index a08de0a..de5b936 100644
>> --- a/tools/perf/arch/x86/util/dwarf-regs.c
>> +++ b/tools/perf/arch/x86/util/dwarf-regs.c
>> @@ -21,55 +21,109 @@
>> */
>>
>> #include <stddef.h>
>> +#include <errno.h> /* for EINVAL */
>> +#include <string.h> /* for strcmp */
>> +#include <linux/ptrace.h> /* for struct pt_regs */
>> +#include <linux/kernel.h> /* for offsetof */
>> #include <dwarf-regs.h>
>>
>> /*
>> - * Generic dwarf analysis helpers
>> + * See arch/x86/kernel/ptrace.c.
>> + * Different from it:
>> + *
>> + * - Since struct pt_regs is defined differently for user and kernel,
>> + * but we want to use 'ax, bx' instead of 'rax, rbx' (which is struct
>> + * field name of user's pt_regs), we make REG_OFFSET_NAME to accept
>> + * both string name and reg field name.
>> + *
>> + * - Since accessing x86_32's pt_regs from x86_64 building is difficult
>> + * and vise versa, we simply fill offset with -1, so
>> + * get_arch_regstr() still works but regs_query_register_offset()
>> + * returns error.
>> + * The only inconvenience caused by it now is that we are not allowed
>> + * to generate BPF prologue for a x86_64 kernel if perf is built for
>> + * x86_32. This is really a rare usecase.
>> + *
>> + * - Order is different from kernel's ptrace.c for get_arch_regstr(), which
>> + * is defined by dwarf.
>> */
>>
>> -#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 pt_regs_offset {
>> + const char *name;
>> + int offset;
>> +};
>> +
>> +#define REG_OFFSET_END {.name = NULL, .offset = 0}
>> +
>> +#ifdef __x86_64__
>> +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
>> +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1}
>> +#else
>> +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1}
>> +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
>> +#endif
>> +
>> +static const struct pt_regs_offset x86_32_regoffset_table[] = {
>> + REG_OFFSET_NAME_32("%ax", eax),
>> + REG_OFFSET_NAME_32("%cx", ecx),
>> + REG_OFFSET_NAME_32("%dx", edx),
>> + REG_OFFSET_NAME_32("%bx", ebx),
>> + REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */
>> + REG_OFFSET_NAME_32("%bp", ebp),
>> + REG_OFFSET_NAME_32("%si", esi),
>> + REG_OFFSET_NAME_32("%di", edi),
>> + REG_OFFSET_END,
>> };
>>
>> -#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",
>> +static const struct pt_regs_offset x86_64_regoffset_table[] = {
>> + REG_OFFSET_NAME_64("%ax", rax),
>> + REG_OFFSET_NAME_64("%dx", rdx),
>> + REG_OFFSET_NAME_64("%cx", rcx),
>> + REG_OFFSET_NAME_64("%bx", rbx),
>> + REG_OFFSET_NAME_64("%si", rsi),
>> + REG_OFFSET_NAME_64("%di", rdi),
>> + REG_OFFSET_NAME_64("%bp", rbp),
>> + REG_OFFSET_NAME_64("%sp", rsp),
>> + REG_OFFSET_NAME_64("%r8", r8),
>> + REG_OFFSET_NAME_64("%r9", r9),
>> + REG_OFFSET_NAME_64("%r10", r10),
>> + REG_OFFSET_NAME_64("%r11", r11),
>> + REG_OFFSET_NAME_64("%r12", r12),
>> + REG_OFFSET_NAME_64("%r13", r13),
>> + REG_OFFSET_NAME_64("%r14", r14),
>> + REG_OFFSET_NAME_64("%r15", r15),
>> + REG_OFFSET_END,
>> };
>>
>> /* 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
>> +#define regoffset_table x86_64_regoffset_table
>> #else
>> -#define ARCH_MAX_REGS X86_32_MAX_REGS
>> -#define arch_regs_table x86_32_regs_table
>> +#define regoffset_table x86_32_regoffset_table
>> #endif
>>
>> +/* 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) ? arch_regs_table[n] : NULL;
>> + return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
>> +}
>> +
>> +/* Reuse code from arch/x86/kernel/ptrace.c */
>> +/**
>> + * regs_query_register_offset() - query register offset from its name
>> + * @name: the name of a register
>> + *
>> + * regs_query_register_offset() returns the offset of a register in struct
>> + * pt_regs from its name. If the name is invalid, this returns -EINVAL;
>> + */
>> +int regs_query_register_offset(const char *name)
>> +{
>> + 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.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists