mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andy Gospodarek <andy@greyhouse.net>
To: David Daney <david.daney@cavium.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mips@linux-mips.org, ralf@linux-mips.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RFC 2/3] samples/bpf: Add define __EMITTING_BPF__ when building BPF
Date: Mon, 19 Jun 2017 10:37:47 -0400	[thread overview]
Message-ID: <20170619143747.GA20370@C02RW35GFVH8.dhcp.broadcom.net> (raw)
In-Reply-To: <20170615223543.22867-3-david.daney@cavium.com>

On Thu, Jun 15, 2017 at 03:35:42PM -0700, David Daney wrote:
> ... this allows gating of inline assembly code that causes llvm to
> fail when emitting BPF.

I floated essentially the same patch in Feb without much luck:

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/492758.html

Many of the folks on the cc-list here had objections then and I suspect they
unfortunately still object.  I like the fact that this fix allowed
architectures that are currently problematic to move forward before there is a
possibly mythical "fix in llvm" to address this problem.

When talking about this in one of the IOVisor calls it was also discussed that
this needs to be fixed for tracing, so there are more than just the BPF
use-case where this is important.

I wasn't sure there was buy-in from the ARM developers, but my thought had been
that a cleaner solution to this would be to reorganize sysreg.h into multiple
files.  The inline assembly would be the only thing in sysreg-asm.h (that was
actually the only thing originally in sysreg.h) and the rest of the code would
be in sysreg.h.

That is not what Dave suggested, but it would be a good starting point for a
custom asm/ layer for BPF/tracing.  I'm with Dave and think a specialized set
of asm/ files for tracing/BPF to avoid these issues all-together and let arch
developers to do whatever they want in their code.

> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  samples/bpf/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index a0561dc762fe..4979e6b56662 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -193,12 +193,12 @@ $(src)/*.c: verify_target_bpf
>  
>  $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
>  
> -# 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.
> +# __EMITTING_BPF__ used to exclude inline assembly, which cannot be
> +# emitted in BPF code.
>  $(obj)/%.o: $(src)/%.c
>  	$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> -		-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> +		-D__KERNEL__ -D__EMITTING_BPF__ \
> +		-Wno-unused-value -Wno-pointer-sign \
>  		-Wno-compare-distinct-pointer-types \
>  		-Wno-gnu-variable-sized-type-not-at-end \
>  		-Wno-address-of-packed-member -Wno-tautological-compare \

  parent reply	other threads:[~2017-06-19 14:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-15 22:35 [PATCH RFC 0/3] bpf/arm64/mips: Avoid inline asm in BPF David Daney
2017-06-15 22:35 ` [PATCH RFC 1/3] arm64: Gate inclusion of asm/sysreg.h by __EMITTING_BPF__ David Daney
2017-06-15 22:35 ` [PATCH RFC 2/3] samples/bpf: Add define __EMITTING_BPF__ when building BPF David Daney
2017-06-16 10:24   ` Daniel Borkmann
2017-06-16 16:14     ` David Miller
2017-06-19 14:37   ` Andy Gospodarek [this message]
2017-06-15 22:35 ` [PATCH RFC 3/3] MIPS: Include file changes to enable building BPF code with llvm David Daney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170619143747.GA20370@C02RW35GFVH8.dhcp.broadcom.net \
    --to=andy@greyhouse.net \
    --cc=ast@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=daniel@iogearbox.net \
    --cc=david.daney@cavium.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=netdev@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®