Re: [PATCH RFC v2 3/5] samples/bpf: Fix inline asm issues building samples on arm64

2017-08-17 Thread Joel Fernandes
On Tue, Aug 8, 2017 at 8:35 PM, David Miller  wrote:
> From: Joel Fernandes 
> Date: Mon, 7 Aug 2017 18:20:49 -0700
>
>> On Mon, Aug 7, 2017 at 11:28 AM, David Miller  wrote:
>>> 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


Re: [PATCH RFC v2 3/5] samples/bpf: Fix inline asm issues building samples on arm64

2017-08-08 Thread David Miller
From: Joel Fernandes 
Date: Mon, 7 Aug 2017 18:20:49 -0700

> On Mon, Aug 7, 2017 at 11:28 AM, David Miller  wrote:
>> 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.


Re: [PATCH RFC v2 3/5] samples/bpf: Fix inline asm issues building samples on arm64

2017-08-07 Thread Joel Fernandes
Hi Dave,

On Mon, Aug 7, 2017 at 11:28 AM, David Miller  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


Re: [PATCH RFC v2 3/5] samples/bpf: Fix inline asm issues building samples on arm64

2017-08-07 Thread David Miller

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.


Re: [PATCH RFC v2 3/5] samples/bpf: Fix inline asm issues building samples on arm64

2017-08-07 Thread Joel Fernandes
On Mon, Aug 7, 2017 at 6:06 AM, Joel Fernandes  wrote:
> 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 
> ---
>  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
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index e5642c8c144d..7591cdd7fe69 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -151,6 +151,8 @@ HOSTLOADLIBES_test_map_in_map += -lelf
>  #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc 
> CLANG=~/git/llvm/build/bin/clang
>  LLC ?= llc
>  CLANG ?= clang
> +PERL ?= perl
> +RM ?= rm
>
>  # Detect that we're cross compiling and use the right compilers and flags
>  ifdef 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 := $(shell dirname $(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



> +   $(RM) $@.tmp.c
> diff --git a/samples/bpf/arm64_asmstubs.h b/samples/bpf/arm64_asmstubs.h
> new file mode 100644
> index ..23d47dbe61b1
> --- /dev/null
> +++ b/samples/bpf/arm64_asmstubs.h
> @@ -0,0 +1,3 @@
> +/* Special handing for current_stack_pointer */
> +#define __ASM_STACK_POINTER_H
> +#define current_stack_pointer 0
> diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
> index 9a9c95f2c9fb..67c9c4438e4b 100644
> --- a/samples/bpf/bpf_helpers.h
> +++ b/samples/bpf/bpf_helpers.h
> @@ -64,12 +64,12 @@ static int (*bpf_xdp_adjust_head)(void *ctx, int offset) =
>   * emit BPF_LD_ABS and BPF_LD_IND instructions
>   */
>  struct sk_buff;
> -unsigned long long load_byte(void *skb,
> -