diff mbox series

[02/13] powerpc32/bpf: Fix codegen for bpf-to-bpf calls

Message ID 52d8fe51f7620a6f27f377791564d79d75463576.1641468127.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive)
State Accepted
Headers show
Series powerpc/bpf: Some fixes and updates | expand

Commit Message

Naveen N. Rao Jan. 6, 2022, 11:45 a.m. UTC
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(+)

Comments

Christophe Leroy Jan. 10, 2022, 9:06 a.m. UTC | #1
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)));
Naveen N. Rao Jan. 10, 2022, 10:52 a.m. UTC | #2
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 mbox series

Patch

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)));