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.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
This opens up the possibility of accessing registers in an arch-independent way. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 2620406a53ec..ad84450e4ab8 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +include ../../../scripts/Makefile.arch LIBDIR := ../../../lib BPFDIR := $(LIBDIR)/bpf @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \ CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \ $(CLANG_SYS_INCLUDES) \ - -Wno-compare-distinct-pointer-types + -Wno-compare-distinct-pointer-types \ + -D__TARGET_ARCH_$(SRCARCH) $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline -- 2.21.0
Also check for __s390__ instead of __s390x__, just in case bpf_helpers.h is ever used by 32-bit userspace. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/bpf_helpers.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h index 5a3d92c8bec8..73071a94769a 100644 --- a/tools/testing/selftests/bpf/bpf_helpers.h +++ b/tools/testing/selftests/bpf/bpf_helpers.h @@ -315,8 +315,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #if defined(__TARGET_ARCH_x86) #define bpf_target_x86 #define bpf_target_defined -#elif defined(__TARGET_ARCH_s930x) - #define bpf_target_s930x +#elif defined(__TARGET_ARCH_s390) + #define bpf_target_s390 #define bpf_target_defined #elif defined(__TARGET_ARCH_arm) #define bpf_target_arm @@ -341,8 +341,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #ifndef bpf_target_defined #if defined(__x86_64__) #define bpf_target_x86 -#elif defined(__s390x__) - #define bpf_target_s930x +#elif defined(__s390__) + #define bpf_target_s390 #elif defined(__arm__) #define bpf_target_arm #elif defined(__aarch64__) @@ -369,7 +369,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #define PT_REGS_SP(x) ((x)->sp) #define PT_REGS_IP(x) ((x)->ip) -#elif defined(bpf_target_s390x) +#elif defined(bpf_target_s390) #define PT_REGS_PARM1(x) ((x)->gprs[2]) #define PT_REGS_PARM2(x) ((x)->gprs[3]) -- 2.21.0
Right now, on certain architectures, these macros are usable only with kernel headers. This patch makes it possible to use them with userspace headers and, as a consequence, not only in BPF samples, but also in BPF selftests. On s390, provide the forward declaration of struct pt_regs and cast it to user_pt_regs in PT_REGS_* macros. This is necessary, because instead of the full struct pt_regs, s390 exposes only its first member user_pt_regs to userspace, and bpf_helpers.h is used with both userspace (in selftests) and kernel (in samples) headers. It was added in commit 466698e654e8 ("s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type"). Ditto on arm64. On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and arm64, x86 provides struct pt_regs to both userspace and kernel, however, with different member names. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/bpf_helpers.h | 75 +++++++++++++++++------ 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h index 73071a94769a..27090d94afb6 100644 --- a/tools/testing/selftests/bpf/bpf_helpers.h +++ b/tools/testing/selftests/bpf/bpf_helpers.h @@ -358,6 +358,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #if defined(bpf_target_x86) +#ifdef __KERNEL__ #define PT_REGS_PARM1(x) ((x)->di) #define PT_REGS_PARM2(x) ((x)->si) #define PT_REGS_PARM3(x) ((x)->dx) @@ -368,19 +369,49 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #define PT_REGS_RC(x) ((x)->ax) #define PT_REGS_SP(x) ((x)->sp) #define PT_REGS_IP(x) ((x)->ip) +#else +#ifdef __i386__ +/* i386 kernel is built with -mregparm=3 */ +#define PT_REGS_PARM1(x) ((x)->eax) +#define PT_REGS_PARM2(x) ((x)->edx) +#define PT_REGS_PARM3(x) ((x)->ecx) +#define PT_REGS_PARM4(x) 0 +#define PT_REGS_PARM5(x) 0 +#define PT_REGS_RET(x) ((x)->esp) +#define PT_REGS_FP(x) ((x)->ebp) +#define PT_REGS_RC(x) ((x)->eax) +#define PT_REGS_SP(x) ((x)->esp) +#define PT_REGS_IP(x) ((x)->eip) +#else +#define PT_REGS_PARM1(x) ((x)->rdi) +#define PT_REGS_PARM2(x) ((x)->rsi) +#define PT_REGS_PARM3(x) ((x)->rdx) +#define PT_REGS_PARM4(x) ((x)->rcx) +#define PT_REGS_PARM5(x) ((x)->r8) +#define PT_REGS_RET(x) ((x)->rsp) +#define PT_REGS_FP(x) ((x)->rbp) +#define PT_REGS_RC(x) ((x)->rax) +#define PT_REGS_SP(x) ((x)->rsp) +#define PT_REGS_IP(x) ((x)->rip) +#endif +#endif #elif defined(bpf_target_s390) -#define PT_REGS_PARM1(x) ((x)->gprs[2]) -#define PT_REGS_PARM2(x) ((x)->gprs[3]) -#define PT_REGS_PARM3(x) ((x)->gprs[4]) -#define PT_REGS_PARM4(x) ((x)->gprs[5]) -#define PT_REGS_PARM5(x) ((x)->gprs[6]) -#define PT_REGS_RET(x) ((x)->gprs[14]) -#define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER */ -#define PT_REGS_RC(x) ((x)->gprs[2]) -#define PT_REGS_SP(x) ((x)->gprs[15]) -#define PT_REGS_IP(x) ((x)->psw.addr) +/* s390 provides user_pt_regs instead of struct pt_regs to userspace */ +struct pt_regs; +#define PT_REGS_S390 const volatile user_pt_regs +#define PT_REGS_PARM1(x) (((PT_REGS_S390 *)(x))->gprs[2]) +#define PT_REGS_PARM2(x) (((PT_REGS_S390 *)(x))->gprs[3]) +#define PT_REGS_PARM3(x) (((PT_REGS_S390 *)(x))->gprs[4]) +#define PT_REGS_PARM4(x) (((PT_REGS_S390 *)(x))->gprs[5]) +#define PT_REGS_PARM5(x) (((PT_REGS_S390 *)(x))->gprs[6]) +#define PT_REGS_RET(x) (((PT_REGS_S390 *)(x))->gprs[14]) +/* Works only with CONFIG_FRAME_POINTER */ +#define PT_REGS_FP(x) (((PT_REGS_S390 *)(x))->gprs[11]) +#define PT_REGS_RC(x) (((PT_REGS_S390 *)(x))->gprs[2]) +#define PT_REGS_SP(x) (((PT_REGS_S390 *)(x))->gprs[15]) +#define PT_REGS_IP(x) (((PT_REGS_S390 *)(x))->psw.addr) #elif defined(bpf_target_arm) @@ -397,16 +428,20 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, #elif defined(bpf_target_arm64) -#define PT_REGS_PARM1(x) ((x)->regs[0]) -#define PT_REGS_PARM2(x) ((x)->regs[1]) -#define PT_REGS_PARM3(x) ((x)->regs[2]) -#define PT_REGS_PARM4(x) ((x)->regs[3]) -#define PT_REGS_PARM5(x) ((x)->regs[4]) -#define PT_REGS_RET(x) ((x)->regs[30]) -#define PT_REGS_FP(x) ((x)->regs[29]) /* Works only with CONFIG_FRAME_POINTER */ -#define PT_REGS_RC(x) ((x)->regs[0]) -#define PT_REGS_SP(x) ((x)->sp) -#define PT_REGS_IP(x) ((x)->pc) +/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */ +struct pt_regs; +#define PT_REGS_ARM64 const volatile struct user_pt_regs +#define PT_REGS_PARM1(x) (((PT_REGS_ARM64 *)(x))->regs[0]) +#define PT_REGS_PARM2(x) (((PT_REGS_ARM64 *)(x))->regs[1]) +#define PT_REGS_PARM3(x) (((PT_REGS_ARM64 *)(x))->regs[2]) +#define PT_REGS_PARM4(x) (((PT_REGS_ARM64 *)(x))->regs[3]) +#define PT_REGS_PARM5(x) (((PT_REGS_ARM64 *)(x))->regs[4]) +#define PT_REGS_RET(x) (((PT_REGS_ARM64 *)(x))->regs[30]) +/* Works only with CONFIG_FRAME_POINTER */ +#define PT_REGS_FP(x) (((PT_REGS_ARM64 *)(x))->regs[29]) +#define PT_REGS_RC(x) (((PT_REGS_ARM64 *)(x))->regs[0]) +#define PT_REGS_SP(x) (((PT_REGS_ARM64 *)(x))->sp) +#define PT_REGS_IP(x) (((PT_REGS_ARM64 *)(x))->pc) #elif defined(bpf_target_mips) -- 2.21.0
Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/testing/selftests/bpf/progs/loop1.c | 2 +- tools/testing/selftests/bpf/progs/loop2.c | 2 +- tools/testing/selftests/bpf/progs/loop3.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c index dea395af9ea9..7cdb7f878310 100644 --- a/tools/testing/selftests/bpf/progs/loop1.c +++ b/tools/testing/selftests/bpf/progs/loop1.c @@ -18,7 +18,7 @@ int nested_loops(volatile struct pt_regs* ctx) for (j = 0; j < 300; j++) for (i = 0; i < j; i++) { if (j & 1) - m = ctx->rax; + m = PT_REGS_RC(ctx); else m = j; sum += i * m; diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c index 0637bd8e8bcf..9b2f808a2863 100644 --- a/tools/testing/selftests/bpf/progs/loop2.c +++ b/tools/testing/selftests/bpf/progs/loop2.c @@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx) int i = 0; while (true) { - if (ctx->rax & 1) + if (PT_REGS_RC(ctx) & 1) i += 3; else i += 7; diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c index 30a0f6cba080..d727657d51e2 100644 --- a/tools/testing/selftests/bpf/progs/loop3.c +++ b/tools/testing/selftests/bpf/progs/loop3.c @@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx) __u64 i = 0, sum = 0; do { i++; - sum += ctx->rax; + sum += PT_REGS_RC(ctx); } while (i < 0x100000000ULL); return sum; } -- 2.21.0
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> > >
On Thu, Jul 11, 2019 at 7:31 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Right now, on certain architectures, these macros are usable only with > kernel headers. This patch makes it possible to use them with userspace > headers and, as a consequence, not only in BPF samples, but also in BPF > selftests. > > On s390, provide the forward declaration of struct pt_regs and cast it > to user_pt_regs in PT_REGS_* macros. This is necessary, because instead > of the full struct pt_regs, s390 exposes only its first member > user_pt_regs to userspace, and bpf_helpers.h is used with both userspace > (in selftests) and kernel (in samples) headers. It was added in commit > 466698e654e8 ("s390/bpf: correct broken uapi for > BPF_PROG_TYPE_PERF_EVENT program type"). > > Ditto on arm64. > > On x86, provide userspace versions of PT_REGS_* macros. Unlike s390 and > arm64, x86 provides struct pt_regs to both userspace and kernel, however, > with different member names. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- This is great, thanks! Acked-by: Andrii Nakryiko <andriin@fb.com> > tools/testing/selftests/bpf/bpf_helpers.h | 75 +++++++++++++++++------ > 1 file changed, 55 insertions(+), 20 deletions(-) > > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h > index 73071a94769a..27090d94afb6 100644 [...]
On Thu, Jul 11, 2019 at 7:31 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Also check for __s390__ instead of __s390x__, just in case bpf_helpers.h > is ever used by 32-bit userspace. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > tools/testing/selftests/bpf/bpf_helpers.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h > index 5a3d92c8bec8..73071a94769a 100644 > --- a/tools/testing/selftests/bpf/bpf_helpers.h > +++ b/tools/testing/selftests/bpf/bpf_helpers.h > @@ -315,8 +315,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, > #if defined(__TARGET_ARCH_x86) > #define bpf_target_x86 > #define bpf_target_defined > -#elif defined(__TARGET_ARCH_s930x) > - #define bpf_target_s930x > +#elif defined(__TARGET_ARCH_s390) > + #define bpf_target_s390 > #define bpf_target_defined > #elif defined(__TARGET_ARCH_arm) > #define bpf_target_arm > @@ -341,8 +341,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, > #ifndef bpf_target_defined > #if defined(__x86_64__) > #define bpf_target_x86 > -#elif defined(__s390x__) > - #define bpf_target_s930x > +#elif defined(__s390__) > + #define bpf_target_s390 > #elif defined(__arm__) > #define bpf_target_arm > #elif defined(__aarch64__) > @@ -369,7 +369,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode, > #define PT_REGS_SP(x) ((x)->sp) > #define PT_REGS_IP(x) ((x)->ip) > > -#elif defined(bpf_target_s390x) > +#elif defined(bpf_target_s390) > > #define PT_REGS_PARM1(x) ((x)->gprs[2]) > #define PT_REGS_PARM2(x) ((x)->gprs[3]) > -- > 2.21.0 >
On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > This opens up the possibility of accessing registers in an > arch-independent way. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/testing/selftests/bpf/Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 2620406a53ec..ad84450e4ab8 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > +include ../../../scripts/Makefile.arch > > LIBDIR := ../../../lib > BPFDIR := $(LIBDIR)/bpf > @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \ > > CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \ > $(CLANG_SYS_INCLUDES) \ > - -Wno-compare-distinct-pointer-types > + -Wno-compare-distinct-pointer-types \ > + -D__TARGET_ARCH_$(SRCARCH) samples/bpf/Makefile uses $(ARCH), why does it work for samples? Should we update samples/bpf/Makefile as well? > > $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline > $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline > -- > 2.21.0 >
> 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.
> Am 12.07.2019 um 02:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> This opens up the possibility of accessing registers in an
>> arch-independent way.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>> tools/testing/selftests/bpf/Makefile | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 2620406a53ec..ad84450e4ab8 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -1,4 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +include ../../../scripts/Makefile.arch
>>
>> LIBDIR := ../../../lib
>> BPFDIR := $(LIBDIR)/bpf
>> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
>>
>> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
>> $(CLANG_SYS_INCLUDES) \
>> - -Wno-compare-distinct-pointer-types
>> + -Wno-compare-distinct-pointer-types \
>> + -D__TARGET_ARCH_$(SRCARCH)
>
> samples/bpf/Makefile uses $(ARCH), why does it work for samples?
> Should we update samples/bpf/Makefile as well?
I believe that in common cases both are okay, but judging by
linux:Makefile and linux:tools/scripts/Makefile.arch, one could use e.g.
ARCH=i686, and that would be converted to SRCARCH=x86. So IMHO SRCARCH
is safer, and we should change bpf/samples/Makefile. I could send a
patch separately.
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!
On Fri, Jul 12, 2019 at 2:00 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> > Am 12.07.2019 um 02:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
> >
> > On Thu, Jul 11, 2019 at 7:32 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >>
> >> This opens up the possibility of accessing registers in an
> >> arch-independent way.
> >>
> >> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> >> ---
> >> tools/testing/selftests/bpf/Makefile | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >> index 2620406a53ec..ad84450e4ab8 100644
> >> --- a/tools/testing/selftests/bpf/Makefile
> >> +++ b/tools/testing/selftests/bpf/Makefile
> >> @@ -1,4 +1,5 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> +include ../../../scripts/Makefile.arch
> >>
> >> LIBDIR := ../../../lib
> >> BPFDIR := $(LIBDIR)/bpf
> >> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
> >>
> >> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >> $(CLANG_SYS_INCLUDES) \
> >> - -Wno-compare-distinct-pointer-types
> >> + -Wno-compare-distinct-pointer-types \
> >> + -D__TARGET_ARCH_$(SRCARCH)
> >
> > samples/bpf/Makefile uses $(ARCH), why does it work for samples?
> > Should we update samples/bpf/Makefile as well?
>
> I believe that in common cases both are okay, but judging by
> linux:Makefile and linux:tools/scripts/Makefile.arch, one could use e.g.
> ARCH=i686, and that would be converted to SRCARCH=x86. So IMHO SRCARCH
> is safer, and we should change bpf/samples/Makefile. I could send a
> patch separately.
Yeah, let's do that then, thanks!