From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752803AbdHNKax (ORCPT ); Mon, 14 Aug 2017 06:30:53 -0400 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 Subject: Re: perf bpf: Reworked fix endianness problem when loading parameters in prologue To: Thomas Richter , , References: <20170814094740.62459-1-tmricht@linux.vnet.ibm.com> CC: , , "Alexei Starovoitov" , Li Zefan , "linux-kernel@vger.kernel.org" From: "Wangnan (F)" Message-ID: Date: Mon, 14 Aug 2017 18:30:20 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170814094740.62459-1-tmricht@linux.vnet.ibm.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.194.139] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090206.59917BD3.000F,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 30db669e797c12e959497fb63411af74 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: 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); >