| Message ID | 52d8fe51f7620a6f27f377791564d79d75463576.1641468127.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | powerpc/bpf: Some fixes and updates | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-PR | fail | merge-conflict |
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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: stable@vger.kernel.org # v5.13+ > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > 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 < 0x2000000 && rel >= -0x2000000) { > 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)));
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
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 < 0x2000000 && rel >= -0x2000000) { 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)));
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: stable@vger.kernel.org # v5.13+ Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- arch/powerpc/net/bpf_jit_comp32.c | 3 +++ 1 file changed, 3 insertions(+)