Re: [PATCH bpf-next v3 4/5] bpf, x86: Add jit support for private stack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 9/26/24 9:58 PM, Leon Hwang wrote:
Hi Yonghong,

A brief review about the usage of this_cpu_off on non-SMP systems.

On 27/9/24 07:45, Yonghong Song wrote:
Add jit support for private stack. For a particular subtree, e.g.,
   subtree_root <== stack depth 120
    subprog1    <== stack depth 80
     subprog2   <== stack depth 40
    subprog3    <== stack depth 160

Let us say that private_stack_ptr is the memory address allocated for
private stack. The frame pointer for each above is calculated like below:
   subtree_root  <== subtree_root_fp = private_stack_ptr + 120
    subprog1     <== subtree_subprog1_fp = subtree_root_fp + 80
     subprog2    <== subtree_subprog2_fp = subtree_subprog1_fp + 40
    subprog3     <== subtree_subprog1_fp = subtree_root_fp + 160

For any function call to helper/kfunc, push/pop prog frame pointer
is needed in order to preserve frame pointer value.

To deal with exception handling, push/pop frame pointer is also used
surrounding call to subsequent subprog. For example,
   subtree_root
    subprog1
      ...
      insn: call bpf_throw
      ...

After jit, we will have
   subtree_root
    insn: push r9
    subprog1
      ...
      insn: push r9
      insn: call bpf_throw
      insn: pop r9
      ...
    insn: pop r9

   exception_handler
      pop r9
      ...
where r9 represents the fp for each subprog.

Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
---
  arch/x86/net/bpf_jit_comp.c | 87 ++++++++++++++++++++++++++++++++++---
  1 file changed, 81 insertions(+), 6 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 06b080b61aa5..c264822c926b 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -325,6 +325,22 @@ struct jit_context {
  /* Number of bytes that will be skipped on tailcall */
  #define X86_TAIL_CALL_OFFSET	(12 + ENDBR_INSN_SIZE)
+static void push_r9(u8 **pprog)
+{
+	u8 *prog = *pprog;
+
+	EMIT2(0x41, 0x51);   /* push r9 */
+	*pprog = prog;
+}
+
+static void pop_r9(u8 **pprog)
+{
+	u8 *prog = *pprog;
+
+	EMIT2(0x41, 0x59);   /* pop r9 */
+	*pprog = prog;
+}
+
  static void push_r12(u8 **pprog)
  {
  	u8 *prog = *pprog;
@@ -491,7 +507,7 @@ static void emit_prologue_tail_call(u8 **pprog, bool is_subprog)
   */
  static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
  			  bool tail_call_reachable, bool is_subprog,
-			  bool is_exception_cb)
+			  bool is_exception_cb, enum bpf_pstack_state  pstack)
  {
  	u8 *prog = *pprog;
@@ -518,6 +534,8 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf,
  		 * first restore those callee-saved regs from stack, before
  		 * reusing the stack frame.
  		 */
+		if (pstack)
+			pop_r9(&prog);
  		pop_callee_regs(&prog, all_callee_regs_used);
  		pop_r12(&prog);
  		/* Reset the stack frame. */
@@ -1404,6 +1422,22 @@ static void emit_shiftx(u8 **pprog, u32 dst_reg, u8 src_reg, bool is64, u8 op)
  	*pprog = prog;
  }
+static void emit_private_frame_ptr(u8 **pprog, void *private_frame_ptr)
+{
+	u8 *prog = *pprog;
+
+	/* movabs r9, private_frame_ptr */
+	emit_mov_imm64(&prog, X86_REG_R9, (long) private_frame_ptr >> 32,
+		       (u32) (long) private_frame_ptr);
+
+	/* add <r9>, gs:[<off>] */
+	EMIT2(0x65, 0x4c);
+	EMIT3(0x03, 0x0c, 0x25);
+	EMIT((u32)(unsigned long)&this_cpu_off, 4);
It should check CONFIG_SMP here like this commit:
1e9e0b85255e ("bpf: handle CONFIG_SMP=n configuration in x86 BPF JIT").

So, it seems better to reuse the code snippet of the commit.

Thanks for pointing this out. I will make the change after waiting some
other reviews.


Thanks,
Leon





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux