Re: [PATCH 2/2] perf tools: Introduce regs_query_register_offset() for arm64
hi 在 2017/1/25 3:09, Arnaldo Carvalho de Melo 写道: Em Tue, Jan 24, 2017 at 06:25:18PM +, Will Deacon escreveu: On Tue, Jan 24, 2017 at 10:30:15AM +, 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 KuangIt 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 2/2] perf tools: Introduce regs_query_register_offset() for arm64
hi 在 2017/1/25 3:09, Arnaldo Carvalho de Melo 写道: Em Tue, Jan 24, 2017 at 06:25:18PM +, Will Deacon escreveu: On Tue, Jan 24, 2017 at 10:30:15AM +, 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 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 2/2] perf tools: Introduce regs_query_register_offset() for arm64
Em Tue, Jan 24, 2017 at 06:25:18PM +, Will Deacon escreveu: > On Tue, Jan 24, 2017 at 10:30:15AM +, 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> 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 2/2] perf tools: Introduce regs_query_register_offset() for arm64
Em Tue, Jan 24, 2017 at 06:25:18PM +, Will Deacon escreveu: > On Tue, Jan 24, 2017 at 10:30:15AM +, 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 > 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 2/2] perf tools: Introduce regs_query_register_offset() for arm64
On Tue, Jan 24, 2017 at 10:30:15AM +, 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> --- > 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 > +#include /* for struct user_pt_regs */ > +#include /* for EINVAL */ > +#include /* for strcmp */ > #include > > -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
On Tue, Jan 24, 2017 at 10:30:15AM +, 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 > --- > 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 > +#include /* for struct user_pt_regs */ > +#include /* for EINVAL */ > +#include /* for strcmp */ > #include > > -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