On Tue, 2022-02-08 at 14:12 -0800, Andrii Nakryiko wrote:
> On Mon, Feb 7, 2022 at 9:16 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> wrote:
> >
> > Andrii says: "... with CO-RE and vmlinux.h it would be more
> > reliable
> > and straightforward to just stick to kernel-internal struct pt_regs
> > everywhere ...".
> >
> > Actually, if vmlinux.h is available, then it's ok to do so for both
> > CO-RE and non-CO-RE cases, since the beginning of struct pt_regs
> > must
> > match (struct) user_pt_regs, which must never change.
> >
> > Implement this by not defining __PT_REGS_CAST if the user included
> > vmlinux.h.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > ---
>
> If we are using CO-RE we don't have to assume vmlinux.h, we can
> define
> our own definition of pt_regs with custom "flavor":
>
> struct pt_regs___s390x {
> long gprs[10];
> long orig_gpr2; /* whatever the right types and names, but order
> doesn't matter */
> } __attribute__((preserve_access_index));
>
>
> And then use `struct pt_regs__s390x` for s390x macros. That way we
> don't assume any specific included header, we have minimal definition
> we need (and it can be different for each architecture. It's still
> CO-RE, still relocatable, and we don't need all these ugly #if
> defined() checks.
I haven't considered using flavors - this will indeed improve the CO-RE
side of things, thanks!
>
> > tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h
> > b/tools/lib/bpf/bpf_tracing.h
> > index 7a015ee8fb11..07e291d77e83 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -118,8 +118,11 @@
> >
> > #define __BPF_ARCH_HAS_SYSCALL_WRAPPER
> >
> > +#if !defined(__KERNEL__) && !defined(__VMLINUX_H__)
> > /* s390 provides user_pt_regs instead of struct pt_regs to
> > userspace */
> > #define __PT_REGS_CAST(x) ((const user_pt_regs *)(x))
> > +#endif
> > +
> > #define __PT_PARM1_REG gprs[2]
> > #define __PT_PARM2_REG gprs[3]
> > #define __PT_PARM3_REG gprs[4]
> > @@ -148,8 +151,11 @@
> >
> > #define __BPF_ARCH_HAS_SYSCALL_WRAPPER
> >
> > +#if !defined(__KERNEL__) && !defined(__VMLINUX_H__)
> > /* arm64 provides struct user_pt_regs instead of struct pt_regs to
> > userspace */
> > #define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
> > +#endif
> > +
> > #define __PT_PARM1_REG regs[0]
> > #define __PT_PARM2_REG regs[1]
> > #define __PT_PARM3_REG regs[2]
> > @@ -207,7 +213,10 @@
> >
> > #elif defined(bpf_target_riscv)
> >
> > +#if !defined(__KERNEL__) && !defined(__VMLINUX_H__)
> > #define __PT_REGS_CAST(x) ((const struct user_regs_struct *)(x))
> > +#endif
> > +
> > #define __PT_PARM1_REG a0
> > #define __PT_PARM2_REG a1
> > #define __PT_PARM3_REG a2
> > --
> > 2.34.1
> >
