Re: [PATCH 23/31] perf tools: Introduce arch_get_reg_info() for x86

[Arnaldo Carvalho de Melo <acme@redhat.com>]

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