[v2] powerpc64/bpf: Support tailcalls with subprogs & BPF exceptions

[PATCH v2 6/6] powerpc64/bpf: Additional NVR handling for bpf_throw

Posted by adubey@linux.ibm.com 3 weeks, 4 days ago

From: Abhishek Dubey <adubey@linux.ibm.com>

The bpf_throw() function never returns, if it has
clobbered any callee-saved register, those will
remain clobbered. The prologue must take care of
saving all callee-saved registers in the frame of
exception boundary program. Later these additional
non volatile registers R14-R25 along with other
NVRs are restored back in the epilogue of exception
callback.

To achieve above objective the frame size is
determined dynamically to accommodate additional
non volatile registers in exception boundary's frame.
For non-exception boundary program, the frame size
remains optimal. The additional instructions to
save & restore r14-r25 registers are emitted only during
exception boundary and exception callback respectively.

Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
---
 arch/powerpc/net/bpf_jit_comp64.c | 70 +++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index a6083dd9786c..941e0818c9ec 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -32,21 +32,37 @@
  *
  *		[	prev sp		] <-------------
  *		[    tail_call_info	] 8		|
- *		[   nv gpr save area	] 6*8		|
+ *		[   nv gpr save area	] 6*8 + (12*8)	|
  *		[    local_tmp_var	] 24		|
  * fp (r31) -->	[   ebpf stack space	] upto 512	|
  *		[     frame header	] 32/112	|
  * sp (r1) --->	[    stack pointer	] --------------
+ *
+ * Additional (12*8) in 'nv gpr save area' only in case of
+ * exception boundary.
  */
 
 /* for bpf JIT code internal usage */
 #define BPF_PPC_STACK_LOCALS	24
+/*
+ * for additional non volatile registers(r14-r25) to be saved
+ * at exception boundary
+ */
+#define BPF_PPC_EXC_STACK_SAVE (12*8)
+
 /* stack frame excluding BPF stack, ensure this is quadword aligned */
 #define BPF_PPC_STACKFRAME	(STACK_FRAME_MIN_SIZE + \
 				 BPF_PPC_STACK_LOCALS + \
 				 BPF_PPC_STACK_SAVE   + \
 				 BPF_PPC_TAILCALL)
 
+/*
+ * same as BPF_PPC_STACKFRAME with save area for additional
+ * non volatile registers saved at exception boundary.
+ * This is quad-word aligned.
+ */
+#define BPF_PPC_EXC_STACKFRAME (BPF_PPC_STACKFRAME + BPF_PPC_EXC_STACK_SAVE)
+
 /* BPF register usage */
 #define TMP_REG_1	(MAX_BPF_JIT_REG + 0)
 #define TMP_REG_2	(MAX_BPF_JIT_REG + 1)
@@ -103,9 +119,12 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
  *		[	  ...       	] 		|
  * sp (r1) --->	[    stack pointer	] --------------
  *		[    tail_call_info	] 8
- *		[   nv gpr save area	] 6*8
+ *		[   nv gpr save area	] 6*8 + (12*8)
  *		[    local_tmp_var	] 24
  *		[   unused red zone	] 224
+ *
+ * Additional (12*8) in 'nv gpr save area' only in case of
+ * exception boundary.
  */
 static int bpf_jit_stack_local(struct codegen_context *ctx)
 {
@@ -114,7 +133,11 @@ static int bpf_jit_stack_local(struct codegen_context *ctx)
 		return STACK_FRAME_MIN_SIZE + ctx->stack_size;
 	} else {
 		/* Stack layout 2 */
-		return -(BPF_PPC_TAILCALL + BPF_PPC_STACK_SAVE + BPF_PPC_STACK_LOCALS);
+		return -(BPF_PPC_TAILCALL
+			+ BPF_PPC_STACK_SAVE
+			+ (ctx->exception_boundary || ctx->exception_cb ?
+							BPF_PPC_EXC_STACK_SAVE:0)
+			+ BPF_PPC_STACK_LOCALS);
 	}
 }
 
@@ -125,9 +148,19 @@ int bpf_jit_stack_tailcallinfo_offset(struct codegen_context *ctx)
 
 static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
 {
-	if (reg >= BPF_PPC_NVR_MIN && reg < 32)
+	int min_valid_nvreg = BPF_PPC_NVR_MIN;
+	/* Default frame size for all cases except exception boundary */
+	int frame_nvr_size = BPF_PPC_STACKFRAME;
+
+	/* Consider all nv regs for handling exceptions */
+	if (ctx->exception_boundary || ctx->exception_cb) {
+		min_valid_nvreg = _R14;
+		frame_nvr_size = BPF_PPC_EXC_STACKFRAME;
+	}
+
+	if (reg >= min_valid_nvreg && reg < 32)
 		return (bpf_has_stack_frame(ctx) ?
-			(BPF_PPC_STACKFRAME + ctx->stack_size) : 0)
+			(frame_nvr_size + ctx->stack_size) : 0)
 				- (8 * (32 - reg)) - BPF_PPC_TAILCALL;
 
 	pr_err("BPF JIT is asking about unknown registers");
@@ -189,7 +222,20 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
 			EMIT(PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF));
 		}
 
-		EMIT(PPC_RAW_STDU(_R1, _R1, -(BPF_PPC_STACKFRAME + ctx->stack_size)));
+		int stack_expand = ctx->exception_boundary || ctx->exception_cb ?
+					BPF_PPC_EXC_STACKFRAME : BPF_PPC_STACKFRAME;
+		EMIT(PPC_RAW_STDU(_R1, _R1, -(stack_expand + ctx->stack_size)));
+	}
+
+	/*
+	 * Program acting as exception boundary pushes R14..R25 in addition to
+	 * BPF callee-saved non volatile registers. Exception callback uses
+	 * the boundary program's stack frame, recover additionally saved
+	 * registers in epilogue of exception callback.
+	 */
+	if (ctx->exception_boundary) {
+		for (i = _R14; i <= _R25; i++)
+			EMIT(PPC_RAW_STD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
 	}
 
 	if (!ctx->exception_cb) {
@@ -237,6 +283,13 @@ static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx
 				bpf_jit_stack_offsetof(ctx, bpf_to_ppc(ARENA_VM_START))));
 
 	if (ctx->exception_cb) {
+		/*
+		 * Recover additionally saved non volatile registers from stack
+		 * frame of exception boundary program.
+		 */
+		for (i = _R14; i <= _R25; i++)
+			EMIT(PPC_RAW_LD(i, _R1, bpf_jit_stack_offsetof(ctx, i)));
+
 		/*
 		 * LR value from boundary-frame is received as second parameter
 		 * in exception callback.
@@ -246,7 +299,10 @@ static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx
 
 	/* Tear down our stack frame */
 	if (bpf_has_stack_frame(ctx)) {
-		EMIT(PPC_RAW_ADDI(_R1, _R1, BPF_PPC_STACKFRAME + ctx->stack_size));
+		int stack_shrink = ctx->exception_cb || ctx->exception_boundary ?
+					BPF_PPC_EXC_STACKFRAME : BPF_PPC_STACKFRAME;
+		EMIT(PPC_RAW_ADDI(_R1, _R1, stack_shrink + ctx->stack_size));
+
 		if (ctx->seen & SEEN_FUNC || ctx->exception_cb) {
 			EMIT(PPC_RAW_LD(_R0, _R1, PPC_LR_STKOFF));
 			EMIT(PPC_RAW_MTLR(_R0));
-- 
2.48.1

Re: [PATCH v2 6/6] powerpc64/bpf: Additional NVR handling for bpf_throw

Posted by Hari Bathini 3 weeks, 1 day ago


On 14/01/26 5:14 pm, adubey@linux.ibm.com wrote:
> From: Abhishek Dubey <adubey@linux.ibm.com>
> 
> The bpf_throw() function never returns, if it has
> clobbered any callee-saved register, those will
> remain clobbered. The prologue must take care of
> saving all callee-saved registers in the frame of
> exception boundary program. Later these additional
> non volatile registers R14-R25 along with other
> NVRs are restored back in the epilogue of exception
> callback.
> 
> To achieve above objective the frame size is
> determined dynamically to accommodate additional
> non volatile registers in exception boundary's frame.
> For non-exception boundary program, the frame size
> remains optimal. The additional instructions to
> save & restore r14-r25 registers are emitted only during
> exception boundary and exception callback respectively.
> 
> Signed-off-by: Abhishek Dubey <adubey@linux.ibm.com>
> ---
>   arch/powerpc/net/bpf_jit_comp64.c | 70 +++++++++++++++++++++++++++----
>   1 file changed, 63 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index a6083dd9786c..941e0818c9ec 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -32,21 +32,37 @@
>    *
>    *		[	prev sp		] <-------------
>    *		[    tail_call_info	] 8		|
> - *		[   nv gpr save area	] 6*8		|
> + *		[   nv gpr save area	] 6*8 + (12*8)	|
>    *		[    local_tmp_var	] 24		|
>    * fp (r31) -->	[   ebpf stack space	] upto 512	|
>    *		[     frame header	] 32/112	|
>    * sp (r1) --->	[    stack pointer	] --------------
> + *
> + * Additional (12*8) in 'nv gpr save area' only in case of
> + * exception boundary.
>    */
>   
>   /* for bpf JIT code internal usage */
>   #define BPF_PPC_STACK_LOCALS	24
> +/*
> + * for additional non volatile registers(r14-r25) to be saved
> + * at exception boundary
> + */
> +#define BPF_PPC_EXC_STACK_SAVE (12*8)
> +
>   /* stack frame excluding BPF stack, ensure this is quadword aligned */
>   #define BPF_PPC_STACKFRAME	(STACK_FRAME_MIN_SIZE + \
>   				 BPF_PPC_STACK_LOCALS + \
>   				 BPF_PPC_STACK_SAVE   + \
>   				 BPF_PPC_TAILCALL)
>   
> +/*
> + * same as BPF_PPC_STACKFRAME with save area for additional
> + * non volatile registers saved at exception boundary.
> + * This is quad-word aligned.
> + */
> +#define BPF_PPC_EXC_STACKFRAME (BPF_PPC_STACKFRAME + BPF_PPC_EXC_STACK_SAVE)
> +
>   /* BPF register usage */
>   #define TMP_REG_1	(MAX_BPF_JIT_REG + 0)
>   #define TMP_REG_2	(MAX_BPF_JIT_REG + 1)
> @@ -103,9 +119,12 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
>    *		[	  ...       	] 		|
>    * sp (r1) --->	[    stack pointer	] --------------
>    *		[    tail_call_info	] 8
> - *		[   nv gpr save area	] 6*8
> + *		[   nv gpr save area	] 6*8 + (12*8)
>    *		[    local_tmp_var	] 24
>    *		[   unused red zone	] 224
> + *
> + * Additional (12*8) in 'nv gpr save area' only in case of
> + * exception boundary.
>    */
>   static int bpf_jit_stack_local(struct codegen_context *ctx)
>   {
> @@ -114,7 +133,11 @@ static int bpf_jit_stack_local(struct codegen_context *ctx)
>   		return STACK_FRAME_MIN_SIZE + ctx->stack_size;
>   	} else {
>   		/* Stack layout 2 */
> -		return -(BPF_PPC_TAILCALL + BPF_PPC_STACK_SAVE + BPF_PPC_STACK_LOCALS);
> +		return -(BPF_PPC_TAILCALL
> +			+ BPF_PPC_STACK_SAVE
> +			+ (ctx->exception_boundary || ctx->exception_cb ?
> +							BPF_PPC_EXC_STACK_SAVE:0)
> +			+ BPF_PPC_STACK_LOCALS);
>   	}
>   }
>   
> @@ -125,9 +148,19 @@ int bpf_jit_stack_tailcallinfo_offset(struct codegen_context *ctx)
>   
>   static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
>   {
> -	if (reg >= BPF_PPC_NVR_MIN && reg < 32)
> +	int min_valid_nvreg = BPF_PPC_NVR_MIN;
> +	/* Default frame size for all cases except exception boundary */
> +	int frame_nvr_size = BPF_PPC_STACKFRAME;
> +
> +	/* Consider all nv regs for handling exceptions */
> +	if (ctx->exception_boundary || ctx->exception_cb) {
> +		min_valid_nvreg = _R14;
> +		frame_nvr_size = BPF_PPC_EXC_STACKFRAME;
> +	}
> +
> +	if (reg >= min_valid_nvreg && reg < 32)
>   		return (bpf_has_stack_frame(ctx) ?
> -			(BPF_PPC_STACKFRAME + ctx->stack_size) : 0)
> +			(frame_nvr_size + ctx->stack_size) : 0)
>   				- (8 * (32 - reg)) - BPF_PPC_TAILCALL;
>   
>   	pr_err("BPF JIT is asking about unknown registers");
> @@ -189,7 +222,20 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
>   			EMIT(PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF));
>   		}
>   

> -		EMIT(PPC_RAW_STDU(_R1, _R1, -(BPF_PPC_STACKFRAME + ctx->stack_size)));
> +		int stack_expand = ctx->exception_boundary || ctx->exception_cb ?
> +					BPF_PPC_EXC_STACKFRAME : BPF_PPC_STACKFRAME;
> +		EMIT(PPC_RAW_STDU(_R1, _R1, -(stack_expand + ctx->stack_size)));

[...]

> -		EMIT(PPC_RAW_ADDI(_R1, _R1, BPF_PPC_STACKFRAME + ctx->stack_size));
> +		int stack_shrink = ctx->exception_cb || ctx->exception_boundary ?
> +					BPF_PPC_EXC_STACKFRAME : BPF_PPC_STACKFRAME;
> +		EMIT(PPC_RAW_ADDI(_R1, _R1, stack_shrink + ctx->stack_size));
> +

An inline helper bpf_jit_stack_size() defined to return the stack
size in both prologue and epilogue while setting up and tearing
down the stack should be more elegant.


Also, IIUC, the JIT code to handle tailcall info is irrelevant for
all subprogs of a BPF program with seen_exception. JIT code in the
prologue for tailcall count handling can be skipped for exception_cb
at least?

- Hari

Re: [PATCH v2 6/6] powerpc64/bpf: Additional NVR handling for bpf_throw

Posted by bot+bpf-ci@kernel.org 3 weeks, 4 days ago

> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index a6083dd9786c..941e0818c9ec 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c

[ ... ]

> @@ -103,9 +119,12 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
>  *		[	  ...       	] 		|
>  * sp (r1) --->	[    stack pointer	] --------------
>  *		[    tail_call_info	] 8
> - *		[   nv gpr save area	] 6*8
> + *		[   nv gpr save area	] 6*8 + (12*8)
>  *		[    local_tmp_var	] 24
>  *		[   unused red zone	] 224
> + *
> + * Additional (12*8) in 'nv gpr save area' only in case of
> + * exception boundary.
>  */
>  static int bpf_jit_stack_local(struct codegen_context *ctx)
>  {
> @@ -114,7 +133,11 @@ static int bpf_jit_stack_local(struct codegen_context *ctx)
>  		return STACK_FRAME_MIN_SIZE + ctx->stack_size;

Does this need to account for BPF_PPC_EXC_STACK_SAVE when
exception_boundary or exception_cb is true?

In the else branch below, the exception case is handled, but the
bpf_has_stack_frame() case returns the same value regardless of
exception_boundary. However, bpf_jit_build_prologue() creates a larger
frame using BPF_PPC_EXC_STACKFRAME when exception_boundary is true.

This inconsistency appears to cause bpf_jit_stack_tailcallinfo_offset()
to return an incorrect offset when both bpf_has_stack_frame() returns
true AND exception_boundary is true. The offset would be off by 96 bytes
(BPF_PPC_EXC_STACK_SAVE).

For example, if a program uses bpf_throw (setting exception_boundary=true)
and also calls helper functions (setting SEEN_FUNC, making
bpf_has_stack_frame() return true), and uses tail calls, the tail call
handling in bpf_jit_emit_tail_call() would access the wrong stack location.

>  	} else {
>  		/* Stack layout 2 */
> -		return -(BPF_PPC_TAILCALL + BPF_PPC_STACK_SAVE + BPF_PPC_STACK_LOCALS);
> +		return -(BPF_PPC_TAILCALL
> +			+ BPF_PPC_STACK_SAVE
> +			+ (ctx->exception_boundary || ctx->exception_cb ?
> +							BPF_PPC_EXC_STACK_SAVE:0)
> +			+ BPF_PPC_STACK_LOCALS);
>  	}
>  }

[ ... ]

> @@ -189,7 +222,20 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
>  			EMIT(PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF));
>  		}
>
> -		EMIT(PPC_RAW_STDU(_R1, _R1, -(BPF_PPC_STACKFRAME + ctx->stack_size)));
> +		int stack_expand = ctx->exception_boundary || ctx->exception_cb ?
> +					BPF_PPC_EXC_STACKFRAME : BPF_PPC_STACKFRAME;
> +		EMIT(PPC_RAW_STDU(_R1, _R1, -(stack_expand + ctx->stack_size)));

The stack frame expansion here uses BPF_PPC_EXC_STACKFRAME for exception
cases, but bpf_jit_stack_local() in the bpf_has_stack_frame() branch does
not account for this larger frame size.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20993216190

[PATCH v2 1/6] powerpc64/bpf: Move tail_call_cnt to bottom of stack frame
[PATCH v2 2/6] powerpc64/bpf: Support tailcalls with subprogs
[PATCH v2 3/6] powerpc64/bpf: Tailcall handling with trampolines
[PATCH v2 4/6] powerpc64/bpf: Add arch_bpf_stack_walk() for BPF JIT
[PATCH v2 5/6] powerpc64/bpf: Support exceptions
[PATCH v2 6/6] powerpc64/bpf: Additional NVR handling for bpf_throw