On Tue, Feb 8, 2022 at 11:46 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
>
> On Tue, 2022-02-08 at 11:25 -0800, Alexei Starovoitov wrote:
> > On Tue, Feb 08, 2022 at 06:16:35AM +0100, Ilya Leoshkevich wrote:
> > > orig_x0's location is used by libbpf tracing macros, therefore it
> > > should not be moved.
> > >
> > > Suggested-by: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
> > > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > > ---
> > > arch/arm64/include/asm/ptrace.h | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/ptrace.h
> > > b/arch/arm64/include/asm/ptrace.h
> > > index 41b332c054ab..7e34c3737839 100644
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -185,6 +185,10 @@ struct pt_regs {
> > > u64 pstate;
> > > };
> > > };
> > > + /*
> > > + * orig_x0 is not exposed via struct user_pt_regs, but its
> > > location is
> > > + * assumed by libbpf's tracing macros, so it should not be
> > > moved.
> > > + */
> >
> > In other words this comment is saying that the layout is ABI.
> > That's not the case. orig_x0 here and equivalent on s390 can be
> > moved.
> > It will break bpf progs written without CO-RE and that is expected.
> > Non CO-RE programs often do all kinds of bpf_probe_read_kernel and
> > will be breaking when kernel layout is changing.
> > I suggest to drop this patch and patch 12.
>
> Yeah, that was the intention here: to promote orig_x0 to ABI using a
> comment, since doing this by extending user_pt_regs turned out to be
> infeasible. I'm actually ok with not doing this, since programs
> compiled with kernel headers and using CO-RE macros will be fine.
The comment like this doesn't convert kernel internal struct into ABI.
The comment is just wrong. BPF progs access many kernel data structs.
s390's and arm64's struct pr_regs is not special in that sense.
It's an internal struct.
> As you say, we don't care about programs that don't use CO-RE too much
> here - if they break after an incompatible kernel change, fine.
Before CO-RE was introduced bpf progs included kernel headers
and were breaking when kernel changes. Nothing new here.
See the history of bcc tools. Some of them are littered
with ifdef VERSION ==.
> The question now is - how much do we care about programs that are
> compiled with userspace headers? Andrii suggested to use offsetofend to
> make syscall macros work there, however, this now requires this ABI
> promotion.
Today s390 and arm64 have user_pt_regs as a first field in pt_regs.
That is kernel internal behavior and that part can change if arch
maintainers have a need for that.
bpf progs without CO-RE would have to be adjusted when kernel changes.
Even with CO-RE it's ok to rename pt_regs->orig_gpr2 on s390.
The progs with CO-RE will break too. The authors of tracing bpf progs
have to expect that sooner or later their progs will break and they
would have to adjust them.
