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,
Leon
