Christophe Leroy wrote:


Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :
In preparation for using kernel TOC, load the same in r2 on entry. With
elfv1, the kernel TOC is already setup by our caller so we just emit a
nop. We adjust the number of instructions to skip on a tail call
accordingly.

Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
---
  arch/powerpc/net/bpf_jit_comp64.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
b/arch/powerpc/net/bpf_jit_comp64.c
index ce4fc59bbd6a92..e05b577d95bf11 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -73,6 +73,12 @@ void bpf_jit_build_prologue(u32 *image, struct 
codegen_context *ctx)
  {
        int i;
+#ifdef PPC64_ELF_ABI_v2
+       PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
+#else
+       EMIT(PPC_RAW_NOP());
+#endif

Can we avoid the #ifdef, using

        if (__is_defined(PPC64_ELF_ABI_v2))
                PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
        else
                EMIT(PPC_RAW_NOP());

Hmm... that doesn't work for me. Is __is_defined() expected to work with macros other than CONFIG options?


+
        /*
         * Initialize tail_call_cnt if we do tail calls.
         * Otherwise, put in NOPs so that it can be skipped when we are
@@ -87,7 +93,7 @@ void bpf_jit_build_prologue(u32 *image, struct 
codegen_context *ctx)
                EMIT(PPC_RAW_NOP());
        }
-#define BPF_TAILCALL_PROLOGUE_SIZE 8
+#define BPF_TAILCALL_PROLOGUE_SIZE     12

Why not change that for v2 ABI only instead of adding a NOP ? ABI won't change during runtime AFAIU

Yeah, I wanted to keep this simple and I felt an additional nop shouldn't matter too much. But, I guess we can get rid of BPF_TAILCALL_PROLOGUE_SIZE since the only user is the function emitting a tail call. I will submit that as a separate cleanup unless I need to redo this series.

Thanks for the reviews!
- Naveen

Reply via email to