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;
Date:   Fri, 25 Feb 2022 13:14:49 +0100
From:   Jiri Olsa <olsajiri@gmail.com>
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     Jiri Olsa <jolsa@kernel.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Ian Rogers <irogers@google.com>,
        "linux-perf-use." <linux-perf-users@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH 3/3] perf tools: Rework prologue generation code
Message-ID: <YhjIOX5BDYh4SRZB@krava>
References: <20220217131916.50615-1-jolsa@kernel.org>
 <20220217131916.50615-4-jolsa@kernel.org>
 <CAEf4BzYP7=JuyuY=xZe71urpxat4ba-JnqeSTcHF=CYmsQbofQ@mail.gmail.com>
 <Yg9geQ0LJjhnrc7j@krava>
 <CAEf4BzZaFWhWf73JbfO7gLi82Nn4ma-qmaZBPij=giNzzoSCTQ@mail.gmail.com>
 <YhJF00d9baPtXjzH@krava>
 <CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAEf4BzaiBO3_617kkXZdYJ8hS8YF--ZLgapNbgeeEJ-pY0H88g@mail.gmail.com>
Precedence: bulk

On Wed, Feb 23, 2022 at 02:29:56PM -0800, Andrii Nakryiko wrote:

SNIP

> > 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
> 
> ugh, perf cornered itself into supporting this crazy scheme and now

well, it just used the interface that was provided at the time

> there is no good solution. I'm still questioning the value of
> supporting this going forward. Is there an evidence that anyone is
> using this functionality at all? Is it worth it trying to carry it on
> just because we have some example that exercises this feature?

yea we discussed this again and I think we can somehow mark this
feature in perf as deprecated and remove it after some time,
because even with the workaround below it'll be pita ;-)

or people will come and scream and we will find some other solution

I already sent the rest of the changes (prog/map priv) separately
and will send some RFC for the deprecation

thanks,
jirka

> 
> Anyways, one way to solve this is to add bpf_program__set_insns() that
> could be called from prog_init_fn callback (which I just realized
> hasn't landed yet, I'll send v4 today) to prepend a simple preamble
> like this:
> 
> r1 = 0;
> r2 = 0;
> r3 = 0;
> f4 = 0;
> r5 = 0; /* how many input arguments we support? */
> 
> This will make all input arguments initialized, libbpf will be able to
> adjust all the relocations and stuff. Once this "prototype program" is
> loaded, perf can grab final instructions and replace first X
> instructions with desired preamble.
> 
> But... ugliness and horror, yeah :(
> 
> 
> >
> > jirka