From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wangnan (F)" Subject: Re: perf bpf: Reworked fix endianness problem when loading parameters in prologue Date: Mon, 14 Aug 2017 18:30:20 +0800 Message-ID: References: <20170814094740.62459-1-tmricht@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from szxga04-in.huawei.com ([45.249.212.190]:3513 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752195AbdHNKav (ORCPT ); Mon, 14 Aug 2017 06:30:51 -0400 In-Reply-To: <20170814094740.62459-1-tmricht@linux.vnet.ibm.com> Sender: linux-perf-users-owner@vger.kernel.org List-ID: To: Thomas Richter , acme@kernel.org, linux-perf-users@vger.kernel.org Cc: brueckner@linux.vnet.ibm.com, zvonko.kosic@de.ibm.com, Alexei Starovoitov , Li Zefan , "linux-kernel@vger.kernel.org" Hi Thomas, Your patch looks good to me. I've tested in my environment and it works. Please resend it to lkml and let Arnaldo to collect it. Thank you. On 2017/8/14 17:47, Thomas Richter wrote: > Perf BPF prologue generator unconditionally fetches 8 bytes for function > parameters, which causes problem on big endian machine. Thomas gives a > detail analysis for this problem: > > http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com > > This patch parses the type of each argument and converts data from > memory to expected type. > > Now the test runs successfully on 4.13.0-rc5: > [root@s8360046 perf]# ./perf test bpf > 38: BPF filter : > 38.1: Basic BPF filtering : Ok > 38.2: BPF pinning : Ok > 38.3: BPF prologue generation : Ok > 38.4: BPF relocation checker : Ok > [root@s8360046 perf]# > > Signed-off-by: Thomas Richter > Signed-off-by: Wang Nan > Cc: Arnaldo Carvalho de Melo > Cc: Thomas Richter > Cc: Alexei Starovoitov > Cc: Hendrik Brueckner > Cc: Li Zefan > --- > tools/perf/util/bpf-prologue.c | 49 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c > index 1356220..827f914 100644 > --- a/tools/perf/util/bpf-prologue.c > +++ b/tools/perf/util/bpf-prologue.c > @@ -58,6 +58,46 @@ check_pos(struct bpf_insn_pos *pos) > return 0; > } > > +/* > + * Convert type string (u8/u16/u32/u64/s8/s16/s32/s64 ..., see > + * Documentation/trace/kprobetrace.txt) to size field of BPF_LDX_MEM > + * instruction (BPF_{B,H,W,DW}). > + */ > +static int > +argtype_to_ldx_size(const char *type) > +{ > + int arg_size = type ? atoi(&type[1]) : 64; > + > + switch (arg_size) { > + case 8: > + return BPF_B; > + case 16: > + return BPF_H; > + case 32: > + return BPF_W; > + case 64: > + default: > + return BPF_DW; > + } > +} > + > +static const char * > +insn_sz_to_str(int insn_sz) > +{ > + switch (insn_sz) { > + case BPF_B: > + return "BPF_B"; > + case BPF_H: > + return "BPF_H"; > + case BPF_W: > + return "BPF_W"; > + case BPF_DW: > + return "BPF_DW"; > + default: > + return "UNKNOWN"; > + } > +} > + > /* Give it a shorter name */ > #define ins(i, p) append_insn((i), (p)) > > @@ -258,9 +298,14 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos, > } > > /* Final pass: read to registers */ > - for (i = 0; i < nargs; i++) > - ins(BPF_LDX_MEM(BPF_DW, BPF_PROLOGUE_START_ARG_REG + i, > + for (i = 0; i < nargs; i++) { > + int insn_sz = (args[i].ref) ? argtype_to_ldx_size(args[i].type) : BPF_DW; > + > + pr_debug("prologue: load arg %d, insn_sz is %s\n", > + i, insn_sz_to_str(insn_sz)); > + ins(BPF_LDX_MEM(insn_sz, BPF_PROLOGUE_START_ARG_REG + i, > BPF_REG_FP, -BPF_REG_SIZE * (i + 1)), pos); > + } > > ins(BPF_JMP_IMM(BPF_JA, BPF_REG_0, 0, JMP_TO_SUCCESS_CODE), pos); >