[PATCH 11/13] powerpc64/bpf elfv2: Setup kernel TOC in r2 on entry

[Christophe Leroy]

Le 11/01/2022 à 11:31, Naveen N. Rao a écrit :
> 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.rao at 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?

Yes, __is_defined() should work with any item.

It is IS_ENABLED() which is supposed to work only with CONFIG options.

See commit 5c189c523e78 ("powerpc/time: Fix mftb()/get_tb() for use with 
the compat VDSO")

Or commit ca5999fde0a1 ("mm: introduce include/linux/pgtable.h")


> 
>>
>>> +
>>>       /*
>>>        * 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.
> 

All this make me think about a discussion I had some time ago with Nic, 
and we ended up trying to get rid of PPC64_ELF_ABI_v2 macro and use a 
CONFIG item instead.

For the result see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/ad0eb12f6e3f49b4a3284fc54c4c4d70c496609e.1634457599.git.christophe.leroy@csgroup.eu/

For the discussion see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4fda65cda906e56aa87806b658e0828c64792403.1634190022.git.christophe.leroy@csgroup.eu/

Christophe