From: Pu Lehui <pulehui@huawei.com>
To: Varun R Mallya <varunrmallya@gmail.com>, <pjw@kernel.org>,
<palmer@dabbelt.com>, <aou@eecs.berkeley.edu>, <ast@kernel.org>,
<daniel@iogearbox.net>, <andrii@kernel.org>, <eddyz87@gmail.com>,
<memxor@gmail.com>, <bjorn@kernel.org>
Cc: <alex@ghiti.fr>, <martin.lau@linux.dev>, <song@kernel.org>,
<yonghong.song@linux.dev>, <jolsa@kernel.org>,
<emil@etsalapatis.com>, <puranjay@kernel.org>,
<linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
<bpf@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH bpf-next v3 4/6] riscv, bpf: Split prologue and epilogue into helper functions
Date: Thu, 30 Jul 2026 17:19:49 +0800 [thread overview]
Message-ID: <bb134352-087d-451f-948a-2c8b635047d0@huawei.com> (raw)
In-Reply-To: <20260722140015.1982158-5-varunrmallya@gmail.com>
Hi Varun,
looks good to me, but some real small nits.
On 2026/7/22 22:00, Varun R Mallya wrote:
> Split bpf_jit_build_prologue() and __build_epilogue() into helpers.
> normal_stack_adjust() computes the frame size for the callee-saved
> registers a program actually clobbers, emit_normal_prologue() allocates
> the frame and spills those registers, and emit_normal_restore() reloads
> them.
>
> The kcfi preamble, fentry nops and tail-call-counter setup are hoisted
> before the frame-size computation and they do not depend on it and the
> emitted instruction sequence is unchanged.
>
> No functional change.
>
> Signed-off-by: Varun R Mallya <varunrmallya@gmail.com>
> ---
> arch/riscv/net/bpf_jit_comp64.c | 152 ++++++++++++++++++--------------
> 1 file changed, 87 insertions(+), 65 deletions(-)
>
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index ad089a9a4ea9..b804382075c4 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -198,9 +198,84 @@ static void emit_imm(u8 rd, s64 val, struct rv_jit_context *ctx)
> emit_addi(rd, rd, lower, ctx);
> }
>
pls use the following comment format:
/*
* xxx
*/
or
/* xxx */
> -static void __build_epilogue(bool is_tail_call, struct rv_jit_context *ctx)
> +/* Stack space for the callee-saved registers a normal program spills,
how about, the registers based strictly on what is seen in the bpf prog.
> + * excluding the BPF stack itself.
> + */
> +static int normal_stack_adjust(struct rv_jit_context *ctx)
> {
> - int stack_adjust = ctx->stack_size, store_offset = stack_adjust - 8;
> + int stack_adjust = 0;
> +
> + if (seen_reg(RV_REG_RA, ctx))
> + stack_adjust += 8;
> + stack_adjust += 8; /* RV_REG_FP */
> + if (seen_reg(RV_REG_S1, ctx))
> + stack_adjust += 8;
> + if (seen_reg(RV_REG_S2, ctx))
> + stack_adjust += 8;
> + if (seen_reg(RV_REG_S3, ctx))
> + stack_adjust += 8;
> + if (seen_reg(RV_REG_S4, ctx))
> + stack_adjust += 8;
> + if (seen_reg(RV_REG_S5, ctx))
> + stack_adjust += 8;
> + if (ctx->arena_vm_start)
> + stack_adjust += 8;
> + stack_adjust += 8; /* RV_REG_TCC */
> +
> + return round_up(stack_adjust, STACK_ALIGN);
> +}
> +
> +/* Allocate the frame and save the callee-saved registers the program
> + * actually clobbers, then point FP at the frame top.
> + */
> +static void emit_normal_prologue(struct rv_jit_context *ctx, int stack_adjust)
prefer to let ctx as last param, and the following pls make a change.
> +{
> + int store_offset = stack_adjust - 8;
> + /* tailcall starts here, emit insn before it must be fixed */
remove this comment.
> +
> + emit_addi(RV_REG_SP, RV_REG_SP, -stack_adjust, ctx);
> +
> + if (seen_reg(RV_REG_RA, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_RA, ctx);
> + store_offset -= 8;
> + }
> + emit_sd(RV_REG_SP, store_offset, RV_REG_FP, ctx);
> + store_offset -= 8;
> + if (seen_reg(RV_REG_S1, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_S1, ctx);
> + store_offset -= 8;
> + }
> + if (seen_reg(RV_REG_S2, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_S2, ctx);
> + store_offset -= 8;
> + }
> + if (seen_reg(RV_REG_S3, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_S3, ctx);
> + store_offset -= 8;
> + }
> + if (seen_reg(RV_REG_S4, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_S4, ctx);
> + store_offset -= 8;
> + }
> + if (seen_reg(RV_REG_S5, ctx)) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_S5, ctx);
> + store_offset -= 8;
> + }
> + if (ctx->arena_vm_start) {
> + emit_sd(RV_REG_SP, store_offset, RV_REG_ARENA, ctx);
> + store_offset -= 8;
> + }
> +
> + /* store TCC from RV_REG_TCC to stack */
> + emit_sd(RV_REG_SP, store_offset, RV_REG_TCC, ctx);
> + ctx->tcc_offset = store_offset;
> +
> + emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust, ctx);
> +}
> +
> +static void emit_normal_restore(struct rv_jit_context *ctx, int stack_adjust) > +{
> + int store_offset = stack_adjust - 8;
>
> if (seen_reg(RV_REG_RA, ctx)) {
> emit_ld(RV_REG_RA, store_offset, RV_REG_SP, ctx);
> @@ -235,6 +310,13 @@ static void __build_epilogue(bool is_tail_call, struct rv_jit_context *ctx)
>
> /* restore TCC from stack to RV_REG_TCC */
> emit_ld(RV_REG_TCC, ctx->tcc_offset, RV_REG_SP, ctx);
> +}
> +
> +static void __build_epilogue(bool is_tail_call, struct rv_jit_context *ctx)
> +{
> + int stack_adjust = ctx->stack_size;
> +
> + emit_normal_restore(ctx, stack_adjust);
>
> emit_addi(RV_REG_SP, RV_REG_SP, stack_adjust, ctx);
> /* Set return value. */
> @@ -2006,34 +2088,12 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>
> void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
> {
> - int i, stack_adjust = 0, store_offset, bpf_stack_adjust;
> + int i, stack_adjust, bpf_stack_adjust;
>
> bpf_stack_adjust = round_up(ctx->prog->aux->stack_depth, STACK_ALIGN);
> if (bpf_stack_adjust)
> mark_fp(ctx);
>
> - if (seen_reg(RV_REG_RA, ctx))
> - stack_adjust += 8;
> - stack_adjust += 8; /* RV_REG_FP */
> - if (seen_reg(RV_REG_S1, ctx))
> - stack_adjust += 8;
> - if (seen_reg(RV_REG_S2, ctx))
> - stack_adjust += 8;
> - if (seen_reg(RV_REG_S3, ctx))
> - stack_adjust += 8;
> - if (seen_reg(RV_REG_S4, ctx))
> - stack_adjust += 8;
> - if (seen_reg(RV_REG_S5, ctx))
> - stack_adjust += 8;
> - if (ctx->arena_vm_start)
> - stack_adjust += 8;
> - stack_adjust += 8; /* RV_REG_TCC */
> -
> - stack_adjust = round_up(stack_adjust, STACK_ALIGN);
> - stack_adjust += bpf_stack_adjust;
> -
> - store_offset = stack_adjust - 8;
> -
> /* emit kcfi type preamble immediately before the first insn */
> emit_kcfi(is_subprog ? cfi_bpf_subprog_hash : cfi_bpf_hash, ctx);
>
> @@ -2046,46 +2106,8 @@ void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
> if (!is_subprog)
> emit(rv_addi(RV_REG_TCC, RV_REG_ZERO, MAX_TAIL_CALL_CNT), ctx);
>
> - /* tailcall starts here, emit insn before it must be fixed */
keep this comment here.
> -
> - emit_addi(RV_REG_SP, RV_REG_SP, -stack_adjust, ctx);
> -
> - if (seen_reg(RV_REG_RA, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_RA, ctx);
> - store_offset -= 8;
> - }
> - emit_sd(RV_REG_SP, store_offset, RV_REG_FP, ctx);
> - store_offset -= 8;
> - if (seen_reg(RV_REG_S1, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_S1, ctx);
> - store_offset -= 8;
> - }
> - if (seen_reg(RV_REG_S2, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_S2, ctx);
> - store_offset -= 8;
> - }
> - if (seen_reg(RV_REG_S3, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_S3, ctx);
> - store_offset -= 8;
> - }
> - if (seen_reg(RV_REG_S4, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_S4, ctx);
> - store_offset -= 8;
> - }
> - if (seen_reg(RV_REG_S5, ctx)) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_S5, ctx);
> - store_offset -= 8;
> - }
> - if (ctx->arena_vm_start) {
> - emit_sd(RV_REG_SP, store_offset, RV_REG_ARENA, ctx);
> - store_offset -= 8;
> - }
> -
> - /* store TCC from RV_REG_TCC to stack */
> - emit_sd(RV_REG_SP, store_offset, RV_REG_TCC, ctx);
> - ctx->tcc_offset = store_offset;
> -
> - emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust, ctx);
> + stack_adjust = normal_stack_adjust(ctx) + bpf_stack_adjust;
> + emit_normal_prologue(ctx, stack_adjust);
>
> if (bpf_stack_adjust)
> emit_addi(RV_REG_S5, RV_REG_SP, bpf_stack_adjust, ctx);
next prev parent reply other threads:[~2026-07-30 9:19 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-22 14:00 [PATCH bpf-next v3 0/6] Add BPF Exceptions support for RISC-V Varun R Mallya
2026-07-22 14:00 ` [PATCH bpf-next v3 1/6] riscv: Use arch_stack_walk() for the kernel callchain Varun R Mallya
2026-07-22 14:00 ` [PATCH bpf-next v3 2/6] riscv, stacktrace: Pass sp and fp to the walk_stackframe() callback Varun R Mallya
2026-07-22 14:00 ` [PATCH bpf-next v3 3/6] riscv, stacktrace: Implement arch_bpf_stack_walk() for BPF Varun R Mallya
2026-07-22 14:00 ` [PATCH bpf-next v3 4/6] riscv, bpf: Split prologue and epilogue into helper functions Varun R Mallya
2026-07-30 9:19 ` Pu Lehui [this message]
2026-07-22 14:00 ` [PATCH bpf-next v3 5/6] riscv, bpf: Add support for BPF exceptions Varun R Mallya
2026-07-22 15:28 ` bot+bpf-ci
2026-07-22 14:00 ` [PATCH bpf-next v3 6/6] riscv, bpf: Remove BPF exceptions from denylist Varun R Mallya
2026-07-30 9:29 ` Pu Lehui
2026-07-24 19:38 ` [PATCH bpf-next v3 0/6] Add BPF Exceptions support for RISC-V Björn Töpel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bb134352-087d-451f-948a-2c8b635047d0@huawei.com \
--to=pulehui@huawei.com \
--cc=alex@ghiti.fr \
--cc=andrii@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=ast@kernel.org \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=palmer@dabbelt.com \
--cc=pjw@kernel.org \
--cc=puranjay@kernel.org \
--cc=song@kernel.org \
--cc=varunrmallya@gmail.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®