On Tue, Jul 2, 2019 at 9:58 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
>
> > Am 02.07.2019 um 18:42 schrieb Y Song <ys114321@xxxxxxxxx>:
> >
> > On Tue, Jul 2, 2019 at 8:40 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
> >>
> >> -#elif defined(__s390x__)
> >> - #define bpf_target_s930x
> >
> > I see in some other places (e.g., bcc) where
> > macro __s390x__ is also used to indicate a s390 architecture.
> > Could you explain the difference between __s390__ and
> > __s390x__?
>
> __s390__ is defined for 32-bit and 64-bit variants, __s390x__ is defined
> for 64-bit variant only.
Thanks.
>
> >> #if defined(bpf_target_x86)
> >>
> >> +#ifdef __KERNEL__
> >
> > In samples/bpf/, __KERNEL__ is defined at clang options and
> > in selftests/bpf/, the __KERNEL__ is not defined.
> >
> > I checked x86 pt_regs definition with and without __KERNEL__.
> > They are identical except some register name difference.
> > I am wondering whether we can unify into all without
> > __KERNEL__. Is __KERNEL__ really needed?
>
> Right now removing it causes the build to fail, but the errors look
> fixable. However, I wonder whether there is a plan regarding this:
> should eBPF programs be built with user headers, kernel headers,
> or both? Status quo appears to be "both", so I’ve decided to stick with
> that in this patch.
Your patch is okay in the sense it maintains the current behavor.
I think it is okay since user level and kernel pt_regs layout are the same
except certain names are different.
>
> >> +/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
> >> +struct pt_regs;
> >> +#define PT_REGS_PARM1(x) (((const volatile user_pt_regs *)(x))->gprs[2])
> >
> > Is user_pt_regs a recent change or has been there for quite some time?
> > I am asking since bcc did not use user_pt_regs yet.
>
> It was added in late 2017 in commit 466698e654e8 ("s390/bpf: correct
> broken uapi for BPF_PROG_TYPE_PERF_EVENT program type“).
Thanks.
