Received-SPF: pass (google.com: domain of linux-kernel+bounces-77671-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240201125225.72796-1-puranjay12@gmail.com> <20240201125225.72796-2-puranjay12@gmail.com>
 <ZdegTX9x2ye-7xIt@arm.com>
In-Reply-To: <ZdegTX9x2ye-7xIt@arm.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Thu, 22 Feb 2024 18:04:35 -0800
Message-ID: <CAADnVQLGGTshMiQAWwJ9UzrEVDR4Z8yk+ki9pUqKLgcH0DRAjA@mail.gmail.com>
Subject: Re: [PATCH bpf-next v3 1/2] arm64: stacktrace: Implement
 arch_bpf_stack_walk() for the BPF JIT
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Puranjay Mohan <puranjay12@gmail.com>, Alexei Starovoitov <ast@kernel.org>, 
	Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, 
	Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, 
	Yonghong Song <yonghong.song@linux.dev>, John Fastabend <john.fastabend@gmail.com>, 
	KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, 
	Jiri Olsa <jolsa@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>, Will Deacon <will@kernel.org>, 
	bpf <bpf@vger.kernel.org>, 
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@vger.kernel.org>, 
	Kumar Kartikeya Dwivedi <memxor@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 22, 2024 at 11:28=E2=80=AFAM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote:
> > This will be used by bpf_throw() to unwind till the program marked as
> > exception boundary and run the callback with the stack of the main
> > program.
> >
> > This is required for supporting BPF exceptions on ARM64.
> >
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> > ---
> >  arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktr=
ace.c
> > index 7f88028a00c0..66cffc5fc0be 100644
> > --- a/arch/arm64/kernel/stacktrace.c
> > +++ b/arch/arm64/kernel/stacktrace.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/efi.h>
> >  #include <linux/export.h>
> > +#include <linux/filter.h>
> >  #include <linux/ftrace.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/sched.h>
> > @@ -266,6 +267,31 @@ noinline noinstr void arch_stack_walk(stack_trace_=
consume_fn consume_entry,
> >       kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs)=
;
> >  }
> >
> > +struct bpf_unwind_consume_entry_data {
> > +     bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp);
> > +     void *cookie;
> > +};
> > +
> > +static bool
> > +arch_bpf_unwind_consume_entry(const struct kunwind_state *state, void =
*cookie)
> > +{
> > +     struct bpf_unwind_consume_entry_data *data =3D cookie;
> > +
> > +     return data->consume_entry(data->cookie, state->common.pc, 0,
> > +                                state->common.fp);
> > +}
> > +
> > +noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *=
cookie, u64 ip, u64 sp,
> > +                                                             u64 fp), =
void *cookie)
> > +{
> > +     struct bpf_unwind_consume_entry_data data =3D {
> > +             .consume_entry =3D consume_entry,
> > +             .cookie =3D cookie,
> > +     };
> > +
> > +     kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current,=
 NULL);
> > +}
>
> Too many "cookies", I found reading this confusing. If you ever respin,
> please use some different "cookie" names.
>
> I guess you want this to be merged via the bpf tree?

We typically take bpf jit patches through bpf-next, since
we do cross arch jits refactoring from time to time,
but nothing like this is pending for this merge window,
so if you want it to go through arm64 tree that's fine with us.

> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thank you for the review!