Re: [PATCH 02/13] powerpc32/bpf: Fix codegen for bpf-to-bpf calls
Christophe Leroy wrote:
Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :
Pad instructions emitted for BPF_CALL so that the number of instructions
generated does not change for different function addresses. This is
especially important for calls to other bpf functions, whose address
will only be known during extra pass.
In first pass, 'image' is NULL and we emit the 4 instructions sequence
already, so the code won't grow after first pass, it can only shrink.
Right, but this patch addresses the scenario where the function address
is only provided during the extra pass. So, even though we will not
write past the end of the BPF image, the emitted instructions can still
be wrong.
On PPC32, a huge effort is made to minimise the situations where 'bl'
cannot be used, see commit 2ec13df16704 ("powerpc/modules: Load modules
closer to kernel text")
And if you take the 8xx for instance, a NOP a just like any other
instruction, it takes one cycle.
If it is absolutely needed, then I'd prefer we use an out-of-line
trampoline for the unlikely case and use 'bl' to that trampoline.
Yes, something like that will be nice to do, but we will still need this
patch for -stable.
The other option is to redo the whole JIT during the extra pass, but
only if we can ensure that we have provisioned for the maximum image
size.
- Naveen
Re: [PATCH 02/13] powerpc32/bpf: Fix codegen for bpf-to-bpf calls
Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :
> Pad instructions emitted for BPF_CALL so that the number of instructions
> generated does not change for different function addresses. This is
> especially important for calls to other bpf functions, whose address
> will only be known during extra pass.
In first pass, 'image' is NULL and we emit the 4 instructions sequence
already, so the code won't grow after first pass, it can only shrink.
On PPC32, a huge effort is made to minimise the situations where 'bl'
cannot be used, see commit 2ec13df16704 ("powerpc/modules: Load modules
closer to kernel text")
And if you take the 8xx for instance, a NOP a just like any other
instruction, it takes one cycle.
If it is absolutely needed, then I'd prefer we use an out-of-line
trampoline for the unlikely case and use 'bl' to that trampoline.
>
> Fixes: 51c66ad849a703 ("powerpc/bpf: Implement extended BPF on PPC32")
> Cc: sta...@vger.kernel.org # v5.13+
> Signed-off-by: Naveen N. Rao
> ---
> arch/powerpc/net/bpf_jit_comp32.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c
> b/arch/powerpc/net/bpf_jit_comp32.c
> index d3a52cd42f5346..997a47fa615b30 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -191,6 +191,9 @@ void bpf_jit_emit_func_call_rel(u32 *image, struct
> codegen_context *ctx, u64 fun
>
> if (image && rel < 0x200 && rel >= -0x200) {
> PPC_BL_ABS(func);
> + EMIT(PPC_RAW_NOP());
> + EMIT(PPC_RAW_NOP());
> + EMIT(PPC_RAW_NOP());
> } else {
> /* Load function address into r0 */
> EMIT(PPC_RAW_LIS(_R0, IMM_H(func)));
[PATCH 02/13] powerpc32/bpf: Fix codegen for bpf-to-bpf calls
Pad instructions emitted for BPF_CALL so that the number of instructions
generated does not change for different function addresses. This is
especially important for calls to other bpf functions, whose address
will only be known during extra pass.
Fixes: 51c66ad849a703 ("powerpc/bpf: Implement extended BPF on PPC32")
Cc: sta...@vger.kernel.org # v5.13+
Signed-off-by: Naveen N. Rao
---
arch/powerpc/net/bpf_jit_comp32.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c
b/arch/powerpc/net/bpf_jit_comp32.c
index d3a52cd42f5346..997a47fa615b30 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -191,6 +191,9 @@ void bpf_jit_emit_func_call_rel(u32 *image, struct
codegen_context *ctx, u64 fun
if (image && rel < 0x200 && rel >= -0x200) {
PPC_BL_ABS(func);
+ EMIT(PPC_RAW_NOP());
+ EMIT(PPC_RAW_NOP());
+ EMIT(PPC_RAW_NOP());
} else {
/* Load function address into r0 */
EMIT(PPC_RAW_LIS(_R0, IMM_H(func)));
--
2.34.1
