Received: by 2002:a05:6a10:7420:0:0:0:0 with SMTP id hk32csp3541182pxb; Sun, 20 Feb 2022 23:41:34 -0800 (PST) X-Google-Smtp-Source: ABdhPJyu19DYfRa0j+2/kcLjSSt5MGEKLv0JTyvbFvB297NpLkzvV+EjbSyTgVh4UC0KBnj5B/ma X-Received: by 2002:a63:a519:0:b0:373:38c9:cea6 with SMTP id n25-20020a63a519000000b0037338c9cea6mr14919608pgf.472.1645429294245; Sun, 20 Feb 2022 23:41:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1645429294; cv=none; d=google.com; s=arc-20160816; b=iUex9dHrzOIn/yvszs5hFrKSzrLNf7N+Nd5J/BWGfuxmIuigN0stPRBNWcScVxcY4M Iddf0gZSeUN2FL3Zs6+Ki2+iQBz4/hUHA6APcBjOPjBB11+LOKnZY9yfUujqXS4eLhh+ v98koH2hshBUl0lfE/hSqjchONJTaRwP5bUtinC2jFnj172wsocBeIyi1mQZnNc+FIK9 MMFrFbE8FcfcZ3awZpGe4A5PKyusXM+aEC6fNy6FJSSc74gIFouFV1G59qFZMxrXYUy4 7QQZqlVWkjaY9AGWU1Cr9+DSok+YWLEhyaITgu9ZJLVp9nbmhefPM6r1+vBAe8U9ZsCA hGVA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=boFnTExG5hKjFOUm8VmDh61ZUOILR6Fg6Ntc7q3DqNw=; b=i8l3YT3h0itb3TSzwKmBSrak0pNMXJ78KGqq03Nh6Zpd3tfkfLwiM/e4padaiB04ew R4PY1VBtSWc82v7W/oV62Lg2oymlDgeThrfpRONyZW8oti/serYLGsxsMUHoBB53fjRw L867pt9lMJyW5f1SvKTLa6A2JDpgBk7LRY3YtKGY5tfPYabM1saVxgKQ1DT0mcao2ic+ 7YOi42Dxg7E/9CGoggmLG8b13TUFDUb9/oIOq3LJVbWSWSlQDTYnYofe9n+KBZ8uCFCQ Upl9iPY0yqhNqQOwMcGYOwNZeO1OHdz334iXorp7jv8dknP6Ju3WMFipzgZ87HhAudAy TUMQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=FoaPdier; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p3si9182583pfh.144.2022.02.20.23.41.20; Sun, 20 Feb 2022 23:41:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=FoaPdier; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235097AbiBTNpT (ORCPT + 99 others); Sun, 20 Feb 2022 08:45:19 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:41476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229884AbiBTNpS (ORCPT ); Sun, 20 Feb 2022 08:45:18 -0500 Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [IPv6:2a00:1450:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A72263980F; Sun, 20 Feb 2022 05:44:56 -0800 (PST) Received: by mail-ej1-x633.google.com with SMTP id gb39so26503496ejc.1; Sun, 20 Feb 2022 05:44:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=boFnTExG5hKjFOUm8VmDh61ZUOILR6Fg6Ntc7q3DqNw=; b=FoaPdier7OaXYRTHjnKlE3uXLOoRUgztcADAko1KPGOIg/+SbimWFmQTl3VHlumTdg ocPjn1/T3n7iu7CAlpPTk9UDJVKL4HXvUv7yjWbIFjvloj2XBRoJ75qpq6sJGxxdcl8J jSmGP01niTz1iNC8hVROGU1/UlGDMXvtZPyxvyj9KNGZ9flLPwQRKRQa8x5ncf+kwDTz X8MB7RjybXfGbArSGZLvPxiJDCGNPBOGjCzZxfs87ob1wW72oS/Cb0P0uz4SMoLGqmBi 7Qd0oaz7LTaPDGZK3lLYDXuk3QHPZpk9+7cFO8pWPbYvDZXwYhnc6t874XVHWURQ/VPS 9ltg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=boFnTExG5hKjFOUm8VmDh61ZUOILR6Fg6Ntc7q3DqNw=; b=MvmP7j4RFqqrxhRXQrosz3BZQFXd0HhY+KuWsgUKY/oGJU9UCvGvydGcu3+69Ys3Rq MCJOhqDhSkWAaJwSK3/mDEkvzewHN9EPLfQzpRAEezAK6ReMxOMXcvOxR9yC4hkM8BLf kFpky9xbMOZs9mTdHRRJwxDv0nksueC4OCyzKNIxja/Rg5UcmmGhLTWwseseFeSIAtJ/ +oNS6YXoyfws7b32TwchYdOe3S4OvQUZ4h5/d8pKSke1tiy23v1FBTYCNYcd+juAVPL8 NV0gm9+5IjhSyyBej5Ntyv6ynKK5P4iyEuK/FqIQKGPp7ztasL9FgizYBPZnI7QDBMpc DzIQ== X-Gm-Message-State: AOAM532jD6OjwutUX++H+0jJKJ9+DxqrIC26iWqI44890gHBLRwFazpO jt/AWJIPK9c+V/P6IiVx2hINccJlW1Xo3g== X-Received: by 2002:a17:906:9d01:b0:6cd:2bfd:6cde with SMTP id fn1-20020a1709069d0100b006cd2bfd6cdemr12572888ejc.478.1645364694884; Sun, 20 Feb 2022 05:44:54 -0800 (PST) Received: from krava ([83.240.63.12]) by smtp.gmail.com with ESMTPSA id s9sm7317901edj.48.2022.02.20.05.44.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 20 Feb 2022 05:44:54 -0800 (PST) Date: Sun, 20 Feb 2022 14:44:51 +0100 From: Jiri Olsa To: Andrii Nakryiko Cc: Jiri Olsa , Arnaldo Carvalho de Melo , Andrii Nakryiko , lkml , Peter Zijlstra , Ingo Molnar , Mark Rutland , Namhyung Kim , Alexander Shishkin , Ian Rogers , "linux-perf-use." , bpf Subject: Re: [PATCH 3/3] perf tools: Rework prologue generation code Message-ID: References: <20220217131916.50615-1-jolsa@kernel.org> <20220217131916.50615-4-jolsa@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 18, 2022 at 11:55:16AM -0800, Andrii Nakryiko wrote: > On Fri, Feb 18, 2022 at 1:01 AM Jiri Olsa wrote: > > > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa wrote: > > > > > > > > Some functions we use now for bpf prologue generation are > > > > going to be deprecated, so reworking the current code not > > > > to use them. > > > > > > > > We need to replace following functions/struct: > > > > bpf_program__set_prep > > > > bpf_program__nth_fd > > > > struct bpf_prog_prep_result > > > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > > before the program is loaded and provide new instructions with > > > > the prologue. > > > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > > for that specific program and load new ebpf programs with prologue > > > > using separate bpf_prog_load calls. > > > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > > private struct. > > > > > > > > Suggested-by: Andrii Nakryiko > > > > Signed-off-by: Jiri Olsa > > > > --- > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > > > > [...] > > > > > > > errout: > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > struct bpf_prog_priv *priv = program_priv(prog); > > > > struct perf_probe_event *pev; > > > > bool need_prologue = false; > > > > - int err, i; > > > > + int i; > > > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > > pr_debug("Internal error when hook preprocessor\n"); > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > return 0; > > > > } > > > > > > > > + /* > > > > + * Do not load programs that need prologue, because we need > > > > + * to add prologue first, check bpf_object__load_prologue. > > > > + */ > > > > + bpf_program__set_autoload(prog, false); > > > > > > if you set autoload to false, program instructions might be invalid in > > > the end. Libbpf doesn't apply some (all?) relocations to such > > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > > "prototypal" BPF program to be loaded before you can grab final > > > instructions. It's not great, but in your case it should work, right? > > > > hum, do we care? it should all be done when the 'new' program with > > the prologue is loaded, right? > > yeah, you should care. If there is any BPF map involved, it is > properly resolved to correct FD (which is put into ldimm64 instruction > in BPF program code) during the load. If program is not autoloaded, > this is skipped. Same for any global variable or subprog call (if it's > not always inlined). So you very much should care for any non-trivial > program. ah too bad.. all that is in the load path, ok > > > > > I switched it off because the verifier failed to load the program > > without the prologue.. because in the original program there's no > > code to grab the arguments that the rest of the code depends on, > > so the verifier sees invalid access > > Do you have an example of C code and corresponding BPF instructions > before/after prologue generation? Just curious to see in details how > this is done. so with following example: SEC("func=do_sched_setscheduler param->sched_priority@user") int bpf_func__setscheduler(void *ctx, int err, int param) { char fmt[] = "prio: %ld"; bpf_trace_printk(fmt, sizeof(fmt), param); return 1; } perf will attach the code to do_sched_setscheduler function, and read 'param->sched_priority' into 'param' argument so the resulting clang object expects 'param' to be in R3 0000000000000000 : 0: b7 01 00 00 64 00 00 00 r1 = 100 1: 6b 1a f8 ff 00 00 00 00 *(u16 *)(r10 - 8) = r1 2: 18 01 00 00 70 72 69 6f 00 00 00 00 3a 20 25 6c r1 = 77926701655 4: 7b 1a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r1 5: bf a1 00 00 00 00 00 00 r1 = r10 6: 07 01 00 00 f0 ff ff ff r1 += -16 7: b7 02 00 00 0a 00 00 00 r2 = 10 8: 85 00 00 00 06 00 00 00 call 6 9: b7 00 00 00 01 00 00 00 r0 = 1 10: 95 00 00 00 00 00 00 00 exit and R3 is loaded in the prologue code (first 15 instructions) and it also sets 'err' (R2) with the result of the reading: 0: (bf) r6 = r1 1: (79) r3 = *(u64 *)(r6 +96) 2: (bf) r7 = r10 3: (07) r7 += -8 4: (7b) *(u64 *)(r10 -8) = r3 5: (b7) r2 = 8 6: (bf) r1 = r7 7: (85) call bpf_probe_read_user#-60848 8: (55) if r0 != 0x0 goto pc+2 9: (61) r3 = *(u32 *)(r10 -8) 10: (05) goto pc+3 11: (b7) r2 = 1 12: (b7) r3 = 0 13: (05) goto pc+1 14: (b7) r2 = 0 15: (bf) r1 = r6 16: (b7) r1 = 100 17: (6b) *(u16 *)(r10 -8) = r1 18: (18) r1 = 0x6c25203a6f697270 20: (7b) *(u64 *)(r10 -16) = r1 21: (bf) r1 = r10 22: (07) r1 += -16 23: (b7) r2 = 10 24: (85) call bpf_trace_printk#-54848 25: (b7) r0 = 1 26: (95) exit I'm still scratching my head how to workaround this.. we do want maps and all the other updates to the code, but verifier won't let it pass without the prologue code jirka