netdev - Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on s390

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1AE29825-8FB2-4682-8822-5F3D16965657@linux.ibm.com>
Date:   Tue, 2 Jul 2019 18:58:34 +0200
From:   Ilya Leoshkevich <iii@...ux.ibm.com>
To:     Y Song <ys114321@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on
 s390

> Am 02.07.2019 um 18:42 schrieb Y Song <ys114321@...il.com>:
> 
> On Tue, Jul 2, 2019 at 8:40 AM Ilya Leoshkevich <iii@...ux.ibm.com> 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.

>> #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.

>> +/* 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“).