| Message ID | 20190711142930.68809-1-iii@linux.ibm.com |
|---|---|
| Headers | show |
| Series | selftests/bpf: fix compiling loop{1,2,3}.c on s390 | expand |
On 07/11, Ilya Leoshkevich wrote: > Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390. > > This patch series consists of three preparatory commits, which make it > possible to use PT_REGS_RC in BPF selftests, followed by the actual fix. > > > > Will this also work for 32-bit x86? > > Thanks, this is a good catch: this builds, but makes 64-bit accesses, as > > if it used the 64-bit variant of pt_regs. I will fix this. > I found four problems in this area: > > 1. Selftest tracing progs are built with -target bpf, leading to struct > pt_regs and friends being interpreted incorrectly. > 2. When the Makefile is adjusted to build them without -target bpf, it > still lacks -m32/-m64, leading to a similar issue. > 3. There is no __i386__ define, leading to incorrect userspace struct > pt_regs variant being chosen for x86. > 4. Finally, there is an issue in my patch: when 1-3 are fixed, it fails > to build, since i386 defines yet another set of field names. > > I will send fixes for problems 1-3 separately, I believe for this patch > series to be correct, it's enough to fix #4 (which I did by adding > another #ifdef). > > I've also changed ARCH to SRCARCH in patch #1, since while ARCH can be > e.g. "i386", SRCARCH always corresponds to directory names under arch/. > > v1->v2: Split into multiple patches. > v2->v3: Added arm64 support. > v3->v4: Added i386 support, use SRCARCH instead of ARCH. Still looks good to me, thanks! Reviewed-by: Stanislav Fomichev <sdf@google.com> Again, should probably go via bpf to fix the existing tests, not bpf-next (but I see bpf tree is not synced with net tree yet). > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > >
> Am 11.07.2019 um 22:35 schrieb Stanislav Fomichev <sdf@fomichev.me>: > > On 07/11, Ilya Leoshkevich wrote: >> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390. >> >> This patch series consists of three preparatory commits, which make it >> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix. >> > Still looks good to me, thanks! > > Reviewed-by: Stanislav Fomichev <sdf@google.com> > > Again, should probably go via bpf to fix the existing tests, not bpf-next > (but I see bpf tree is not synced with net tree yet). Sorry, I missed your comment the last time. You are right - that’s the reason I’ve been sending this to bpf-next so far — loop*.c don’t even exist in the bpf tree.
On 07/12/2019 10:55 AM, Ilya Leoshkevich wrote: >> Am 11.07.2019 um 22:35 schrieb Stanislav Fomichev <sdf@fomichev.me>: >> >> On 07/11, Ilya Leoshkevich wrote: >>> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390. >>> >>> This patch series consists of three preparatory commits, which make it >>> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix. >>> >> Still looks good to me, thanks! >> >> Reviewed-by: Stanislav Fomichev <sdf@google.com> >> >> Again, should probably go via bpf to fix the existing tests, not bpf-next >> (but I see bpf tree is not synced with net tree yet). > > Sorry, I missed your comment the last time. You are right - that’s the > reason I’ve been sending this to bpf-next so far — loop*.c don’t even > exist in the bpf tree. Applied to bpf tree (and also added Stanislav's Tested-by to the last one), thanks!