From: Joel Fernandes <hidden> Date: 2017-08-07 13:07:09
inline assembly has haunted building samples on arm64 for quite sometime.
This patch uses the pre-processor to noop all occurences of inline asm when
compiling the BPF sample for the BPF target.
This patch reintroduces inclusion of asm/sysregs.h which needs to be included
to avoid compiler errors now, see [1]. Previously a hack prevented this
inclusion [2] (to avoid the exact problem this patch fixes - skipping inline
assembler) but the hack causes other errors now and no longer works.
Using the preprocessor to noop the inline asm occurences, we also avoid
any future unstable hackery needed (such as those that skip asm headers)
and provides information that asm headers may have which could have been
used but which the inline asm issues prevented. This is the least messy
of all hacks in my opinion.
[1] https://lkml.org/lkml/2017/8/5/143
[2] https://lists.linaro.org/pipermail/linaro-kernel/2015-November/024036.html
Signed-off-by: Joel Fernandes <redacted>
---
samples/bpf/Makefile | 40 +++++++++++++++++++++++++++++++++-------
samples/bpf/arm64_asmstubs.h | 3 +++
samples/bpf/bpf_helpers.h | 12 ++++++------
samples/bpf/generic_asmstubs.h | 4 ++++
4 files changed, 46 insertions(+), 13 deletions(-)
create mode 100644 samples/bpf/arm64_asmstubs.h
create mode 100644 samples/bpf/generic_asmstubs.h
@@ -151,6 +151,8 @@ HOSTLOADLIBES_test_map_in_map += -lelf# make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clangLLC?=llcCLANG?=clang+PERL?=perl+RM?=rm# Detect that we're cross compiling and use the right compilers and flagsifdef CROSS_COMPILE
@@ -186,14 +188,38 @@ verify_target_bpf: verify_cmds$(src)/*.c:verify_target_bpf-# asm/sysreg.h - inline assembly used by it is incompatible with llvm.-# But, there is no easy way to fix it, so just exclude it since it is-# useless for BPF samples.-$(obj)/%.o:$(src)/%.c-$(CLANG)$(NOSTDINC_FLAGS)$(LINUXINCLUDE)$(EXTRA_CFLAGS)\--D__KERNEL__-D__ASM_SYSREG_H-Wno-unused-value-Wno-pointer-sign\+curdir:=$(shelldirname$(realpath$(lastword$(MAKEFILE_LIST))))+ifeq ($(wildcard $(curdir)/${ARCH}_asmstubs.h),)+ARCH_ASM_STUBS:=+else+ARCH_ASM_STUBS:=-include$(src)/${ARCH}_asmstubs.h+endif++ASM_STUBS:=${ARCH_ASM_STUBS}-include$(src)/generic_asmstubs.h++CLANG_ARGS=$(NOSTDINC_FLAGS)$(LINUXINCLUDE)$(EXTRA_CFLAGS)\+-D__KERNEL__-Wno-unused-value-Wno-pointer-sign\+$(ASM_STUBS)\-Wno-compare-distinct-pointer-types\-Wno-gnu-variable-sized-type-not-at-end\-Wno-address-of-packed-member-Wno-tautological-compare\-Wno-unknown-warning-option\--O2-emit-llvm-c$<-o-|$(LLC)-march=bpf-filetype=obj-o$@+-O2-emit-llvm++$(obj)/%.o:$(src)/%.c+ # Steps to compile BPF sample while getting rid of inline asm+ # This has the advantage of not having to skip important asm headers+ # Step 1. Use clang preprocessor to stub out asm() calls+ # Step 2. Replace all "asm volatile" with single keyword "asmvolatile"+ # Step 3. Use clang preprocessor to noop all asm volatile() calls+ # and restore asm_bpf to asm for BPF's asm directives+ # Step 4. Compile and link++$(CLANG)-E$(CLANG_ARGS)-c$<-o-|\+$(PERL)-pe"s/[_\s]*asm[_\s]*volatile[_\s]*/asmvolatile/g"|\+$(CLANG)-E$(ASM_STUBS)--o-|\+$(CLANG)-E-Dasm_bpf=asm--o$@.tmp.c++$(CLANG)$(CLANG_ARGS)-c$@.tmp.c\+-o-|$(LLC)-march=bpf-filetype=obj-o$@+$(RM)$@.tmp.c
@@ -64,12 +64,12 @@ static int (*bpf_xdp_adjust_head)(void *ctx, int offset) =*emitBPF_LD_ABSandBPF_LD_INDinstructions*/structsk_buff;-unsignedlonglongload_byte(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.byte");-unsignedlonglongload_half(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.half");-unsignedlonglongload_word(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.word");+unsignedlonglongload_byte(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.byte");+unsignedlonglongload_half(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.half");+unsignedlonglongload_word(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.word");/* a helper structure used by eBPF C program*todescribemapattributestoelf_bpfloader
From: Joel Fernandes <hidden> Date: 2017-08-07 13:15:09
On Mon, Aug 7, 2017 at 6:06 AM, Joel Fernandes [off-list ref] wrote:
quoted hunk
inline assembly has haunted building samples on arm64 for quite sometime.
This patch uses the pre-processor to noop all occurences of inline asm when
compiling the BPF sample for the BPF target.
This patch reintroduces inclusion of asm/sysregs.h which needs to be included
to avoid compiler errors now, see [1]. Previously a hack prevented this
inclusion [2] (to avoid the exact problem this patch fixes - skipping inline
assembler) but the hack causes other errors now and no longer works.
Using the preprocessor to noop the inline asm occurences, we also avoid
any future unstable hackery needed (such as those that skip asm headers)
and provides information that asm headers may have which could have been
used but which the inline asm issues prevented. This is the least messy
of all hacks in my opinion.
[1] https://lkml.org/lkml/2017/8/5/143
[2] https://lists.linaro.org/pipermail/linaro-kernel/2015-November/024036.html
Signed-off-by: Joel Fernandes <redacted>
---
samples/bpf/Makefile | 40 +++++++++++++++++++++++++++++++++-------
samples/bpf/arm64_asmstubs.h | 3 +++
samples/bpf/bpf_helpers.h | 12 ++++++------
samples/bpf/generic_asmstubs.h | 4 ++++
4 files changed, 46 insertions(+), 13 deletions(-)
create mode 100644 samples/bpf/arm64_asmstubs.h
create mode 100644 samples/bpf/generic_asmstubs.h
@@ -151,6 +151,8 @@ HOSTLOADLIBES_test_map_in_map += -lelf# make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clangLLC?=llcCLANG?=clang+PERL?=perl+RM?=rm# Detect that we're cross compiling and use the right compilers and flagsifdef CROSS_COMPILE
@@ -186,14 +188,38 @@ verify_target_bpf: verify_cmds$(src)/*.c:verify_target_bpf-# asm/sysreg.h - inline assembly used by it is incompatible with llvm.-# But, there is no easy way to fix it, so just exclude it since it is-# useless for BPF samples.-$(obj)/%.o:$(src)/%.c-$(CLANG)$(NOSTDINC_FLAGS)$(LINUXINCLUDE)$(EXTRA_CFLAGS)\--D__KERNEL__-D__ASM_SYSREG_H-Wno-unused-value-Wno-pointer-sign\+curdir:=$(shelldirname$(realpath$(lastword$(MAKEFILE_LIST))))+ifeq ($(wildcard $(curdir)/${ARCH}_asmstubs.h),)+ARCH_ASM_STUBS:=+else+ARCH_ASM_STUBS:=-include$(src)/${ARCH}_asmstubs.h+endif++ASM_STUBS:=${ARCH_ASM_STUBS}-include$(src)/generic_asmstubs.h++CLANG_ARGS=$(NOSTDINC_FLAGS)$(LINUXINCLUDE)$(EXTRA_CFLAGS)\+-D__KERNEL__-Wno-unused-value-Wno-pointer-sign\+$(ASM_STUBS)\-Wno-compare-distinct-pointer-types\-Wno-gnu-variable-sized-type-not-at-end\-Wno-address-of-packed-member-Wno-tautological-compare\-Wno-unknown-warning-option\--O2-emit-llvm-c$<-o-|$(LLC)-march=bpf-filetype=obj-o$@+-O2-emit-llvm++$(obj)/%.o:$(src)/%.c+ # Steps to compile BPF sample while getting rid of inline asm+ # This has the advantage of not having to skip important asm headers+ # Step 1. Use clang preprocessor to stub out asm() calls+ # Step 2. Replace all "asm volatile" with single keyword "asmvolatile"+ # Step 3. Use clang preprocessor to noop all asm volatile() calls+ # and restore asm_bpf to asm for BPF's asm directives+ # Step 4. Compile and link++$(CLANG)-E$(CLANG_ARGS)-c$<-o-|\+$(PERL)-pe"s/[_\s]*asm[_\s]*volatile[_\s]*/asmvolatile/g"|\+$(CLANG)-E$(ASM_STUBS)--o-|\+$(CLANG)-E-Dasm_bpf=asm--o$@.tmp.c++$(CLANG)$(CLANG_ARGS)-c$@.tmp.c\+-o-|$(LLC)-march=bpf-filetype=obj-o$@
Just found an issue here that asm_bpf will be stubbed out by
CLANG_ARGS, will fix it next rev. I'll also try to do object
inspection to make sure the asm directive bpf_helpers is working.
thanks!
-Joel
@@ -64,12 +64,12 @@ static int (*bpf_xdp_adjust_head)(void *ctx, int offset) =*emitBPF_LD_ABSandBPF_LD_INDinstructions*/structsk_buff;-unsignedlonglongload_byte(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.byte");-unsignedlonglongload_half(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.half");-unsignedlonglongload_word(void*skb,-unsignedlonglongoff)asm("llvm.bpf.load.word");+unsignedlonglongload_byte(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.byte");+unsignedlonglongload_half(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.half");+unsignedlonglongload_word(void*skb,unsignedlonglongoff)+asm_bpf("llvm.bpf.load.word");/* a helper structure used by eBPF C program*todescribemapattributestoelf_bpfloader
From: David Miller <davem@davemloft.net> Date: 2017-08-07 18:28:43
Please, no.
The amount of hellish hacks we are adding to deal with this is getting
way out of control.
BPF programs MUST have their own set of asm headers, this is the
only way to get around this issue in the long term.
I am also strongly against adding -static to the build.
From: Joel Fernandes <hidden> Date: 2017-08-08 01:20:52
Hi Dave,
On Mon, Aug 7, 2017 at 11:28 AM, David Miller [off-list ref] wrote:
Please, no.
Sorry you dislike it, I had intentionally marked it as RFC as its an
idea I was just toying with the idea and posted it early to get
feedback.
The amount of hellish hacks we are adding to deal with this is getting
way out of control.
I agree with you that hellish hacks are being added which is why it
keeps breaking. I think one of the things my series does is to add
back inclusion of asm headers that were previously removed (that is
the worst hellish hack in my opinion that existing in mainline). So in
that respect my patch is an improvement and makes it possible to build
for arm64 platforms (which is currently broken in mainline).
BPF programs MUST have their own set of asm headers, this is the
only way to get around this issue in the long term.
Wouldn't that break scripts or bpf code that instruments/trace arch
specific code?
I am also strongly against adding -static to the build.
I can drop -static if you prefer, that's not an issue.
As I understand it, there are no other cleaner alternatives and this
patchset makes the samples work. I would even argue that's its more
functional than previous attempts and fixes something broken in
mainline in a more generic way. If you can provide an example of where
my patchset may not work, I would love to hear it. My whole idea was
to do it in a way that makes future breakage not happen. I don't think
that leaving things broken in this state for extended periods of time
makes sense and IMHO will slow usage of bpf samples on other
platforms.
thanks,
-Joel
From: David Miller <davem@davemloft.net> Date: 2017-08-09 03:35:45
From: Joel Fernandes <redacted>
Date: Mon, 7 Aug 2017 18:20:49 -0700
On Mon, Aug 7, 2017 at 11:28 AM, David Miller [off-list ref] wrote:
quoted
The amount of hellish hacks we are adding to deal with this is getting
way out of control.
I agree with you that hellish hacks are being added which is why it
keeps breaking. I think one of the things my series does is to add
back inclusion of asm headers that were previously removed (that is
the worst hellish hack in my opinion that existing in mainline). So in
that respect my patch is an improvement and makes it possible to build
for arm64 platforms (which is currently broken in mainline).
Yeah that is a problem.
Perhaps another avenue of attack is to separate "type" header files from
stuff that has functiond declarations and inline assembler code.
From: Joel Fernandes <hidden> Date: 2017-08-18 06:55:42
On Tue, Aug 8, 2017 at 8:35 PM, David Miller [off-list ref] wrote:
From: Joel Fernandes <redacted>
Date: Mon, 7 Aug 2017 18:20:49 -0700
quoted
On Mon, Aug 7, 2017 at 11:28 AM, David Miller [off-list ref] wrote:
quoted
The amount of hellish hacks we are adding to deal with this is getting
way out of control.
I agree with you that hellish hacks are being added which is why it
keeps breaking. I think one of the things my series does is to add
back inclusion of asm headers that were previously removed (that is
the worst hellish hack in my opinion that existing in mainline). So in
that respect my patch is an improvement and makes it possible to build
for arm64 platforms (which is currently broken in mainline).
Yeah that is a problem.
Perhaps another avenue of attack is to separate "type" header files from
stuff that has functiond declarations and inline assembler code.
I was thinking that's probably a huge undertaking if you meant doing
the above for every arch?
Also another way could be to modify clang to ignore inline asm
directives during compilation? Do you have any comments about such
approach?
thanks,
-Joel