All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: James Hilliard <james.hilliard1@gmail.com>
Cc: bpf@vger.kernel.org, Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Tom Rix <trix@redhat.com>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH] libbpf: add GCC support for bpf_tail_call_static
Date: Mon, 29 Aug 2022 22:19:52 +0200	[thread overview]
Message-ID: <87czcissyf.fsf@oracle.com> (raw)
In-Reply-To: <20220829195349.706672-1-james.hilliard1@gmail.com> (James Hilliard's message of "Mon, 29 Aug 2022 13:53:49 -0600")


Hi James.

> The bpf_tail_call_static function is currently not defined unless
> using clang >= 8.
>
> To support bpf_tail_call_static on GCC we can check if __clang__ is
> not defined to enable bpf_tail_call_static.
>
> We also need to check for the GCC style __BPF__ in addition to __bpf__
> for this to work as GCC does not define __bpf__.

No need for that complication.  I just pushed a patch to GCC that makes
it define __bpf__ as a target macro, in addition to __BPF__, like LLVM
does.

https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600534.html

> We need to use GCC assembly syntax when the compiler does not define
> __clang__ as LLVM inline assembly is not fully compatible with GCC.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  tools/lib/bpf/bpf_helpers.h | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 7349b16b8e2f..a0650b840cda 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -131,7 +131,7 @@
>  /*
>   * Helper function to perform a tail call with a constant/immediate map slot.
>   */
> -#if __clang_major__ >= 8 && defined(__bpf__)
> +#if (!defined(__clang__) || __clang_major__ >= 8) && (defined(__bpf__) || defined(__BPF__))
>  static __always_inline void
>  bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
>  {
> @@ -139,8 +139,8 @@ bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
>  		__bpf_unreachable();
>  
>  	/*
> -	 * Provide a hard guarantee that LLVM won't optimize setting r2 (map
> -	 * pointer) and r3 (constant map index) from _different paths_ ending
> +	 * Provide a hard guarantee that the compiler won't optimize setting r2
> +	 * (map pointer) and r3 (constant map index) from _different paths_ ending
>  	 * up at the _same_ call insn as otherwise we won't be able to use the
>  	 * jmpq/nopl retpoline-free patching by the x86-64 JIT in the kernel
>  	 * given they mismatch. See also d2e4c1e6c294 ("bpf: Constant map key
> @@ -148,12 +148,19 @@ bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
>  	 *
>  	 * Note on clobber list: we need to stay in-line with BPF calling
>  	 * convention, so even if we don't end up using r0, r4, r5, we need
> -	 * to mark them as clobber so that LLVM doesn't end up using them
> -	 * before / after the call.
> +	 * to mark them as clobber so that the compiler doesn't end up using
> +	 * them before / after the call.
>  	 */
> -	asm volatile("r1 = %[ctx]\n\t"
> +	asm volatile(
> +#ifdef __clang__
> +		     "r1 = %[ctx]\n\t"
>  		     "r2 = %[map]\n\t"
>  		     "r3 = %[slot]\n\t"
> +#else
> +		     "mov %%r1,%[ctx]\n\t"
> +		     "mov %%r2,%[map]\n\t"
> +		     "mov %%r3,%[slot]\n\t"
> +#endif
>  		     "call 12"
>  		     :: [ctx]"r"(ctx), [map]"r"(map), [slot]"i"(slot)
>  		     : "r0", "r1", "r2", "r3", "r4", "r5");

      reply	other threads:[~2022-08-29 20:21 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-29 19:53 [PATCH] libbpf: add GCC support for bpf_tail_call_static James Hilliard
2022-08-29 20:19 ` Jose E. Marchesi [this message]

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=87czcissyf.fsf@oracle.com \
    --to=jose.marchesi@oracle.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=james.hilliard1@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=martin.lau@linux.dev \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=trix@redhat.com \
    --cc=yhs@fb.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.