On Wed, Oct 11, 2023 at 11:27:22PM +0800, Leon Hwang wrote: > For next commit to reuse emit_nops(), move emit_nops() before > emit_prologue(). > > By the way, change memcpy(prog, x86_nops[5], X86_PATCH_SIZE) to > emit_nops(&prog, X86_PATCH_SIZE). I find the subject of the commit a bit bogus. Could you change it to something like: use emit_nops() to produce nop5 instead memcpy'ing x86_nops[5] I also feel that you should be consistent and address other spots that are the same as the one that you are touching in emit_prologue() - there are two more from what i see. > > Signed-off-by: Leon Hwang <hffilwlqm@xxxxxxxxx> > --- > arch/x86/net/bpf_jit_comp.c | 41 ++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 21 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 8c10d9abc2394..c2a0465d37da4 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -304,6 +304,25 @@ static void pop_callee_regs(u8 **pprog, bool *callee_regs_used) > *pprog = prog; > } > > +static void emit_nops(u8 **pprog, int len) > +{ > + u8 *prog = *pprog; > + int i, noplen; > + > + while (len > 0) { > + noplen = len; > + > + if (noplen > ASM_NOP_MAX) > + noplen = ASM_NOP_MAX; > + > + for (i = 0; i < noplen; i++) > + EMIT1(x86_nops[noplen][i]); > + len -= noplen; > + } > + > + *pprog = prog; > +} > + > /* > * Emit x86-64 prologue code for BPF program. > * bpf_tail_call helper will skip the first X86_TAIL_CALL_OFFSET bytes > @@ -319,8 +338,7 @@ static void emit_prologue(u8 **pprog, u32 stack_depth, bool ebpf_from_cbpf, > * but let's waste 5 bytes for now and optimize later > */ > EMIT_ENDBR(); > - memcpy(prog, x86_nops[5], X86_PATCH_SIZE); > - prog += X86_PATCH_SIZE; > + emit_nops(&prog, X86_PATCH_SIZE); > if (!ebpf_from_cbpf) { > if (tail_call_reachable && !is_subprog) > /* When it's the entry of the whole tailcall context, > @@ -989,25 +1007,6 @@ static void detect_reg_usage(struct bpf_insn *insn, int insn_cnt, > } > } > > -static void emit_nops(u8 **pprog, int len) > -{ > - u8 *prog = *pprog; > - int i, noplen; > - > - while (len > 0) { > - noplen = len; > - > - if (noplen > ASM_NOP_MAX) > - noplen = ASM_NOP_MAX; > - > - for (i = 0; i < noplen; i++) > - EMIT1(x86_nops[noplen][i]); > - len -= noplen; > - } > - > - *pprog = prog; > -} > - > /* emit the 3-byte VEX prefix > * > * r: same as rex.r, extra bit for ModRM reg field > -- > 2.41.0 >