Re: [PATCH bpf-next v11 03/10] bpf: Introduce global percpu data

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2026-08-11 at 12:16 +0800, Leon Hwang wrote:
> On 11/8/26 07:22, Eduard Zingerman wrote:
> > On Fri, 2026-08-07 at 00:31 +0800, Leon Hwang wrote:
> > 
> > ...
> > 
> > > diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> > > index 440e73e1b11b..f2398749103b 100644
> > > --- a/kernel/bpf/fixups.c
> > > +++ b/kernel/bpf/fixups.c
> > > @@ -1817,6 +1817,43 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
> > >  			goto next_insn;
> > >  		}
> > >  
> > > +		if (bpf_jit_supports_percpu_insn() &&
> > > +		    insn->code == (BPF_LD | BPF_IMM | BPF_DW) &&
> > > +		    (insn->src_reg == BPF_PSEUDO_MAP_VALUE ||
> > > +		     insn->src_reg == BPF_PSEUDO_MAP_IDX_VALUE)) {
> > > +			struct bpf_map *map;
> > > +
> > > +			aux = &env->insn_aux_data[i + delta];
> > > +			map = env->used_maps[aux->map_index];
> > > +			if (map->map_type != BPF_MAP_TYPE_PERCPU_ARRAY)
> > > +				goto next_insn;
> > > +
> > > +			prog->jit_required = true;
> > > +
> > > +			/*
> > > +			 * We are *skipping* first half of ld_imm64 insn
> > > +			 * with 'i++;', patching over second half of it
> > > +			 * with that same half + mov64_percpu_reg insn.
> > > +			 * All because bpf_patch_insn_data() can only
> > > +			 * replace one 8-byte insn, which does not work
> > > +			 * well for ld_imm64 insn.
> > > +			 */
> > > +
> > > +			insn_buf[0] = insn[1];
> > > +			insn_buf[1] = BPF_MOV64_PERCPU_REG(insn->dst_reg, insn->dst_reg);
> > > +			cnt = 2;
> > 
> > Hi Leon,
> > 
> > Sorry for joining the discussion so late, but Andrii asked me to take
> > a look at the verifier part of the changes. Could you please elaborate
> > on what exactly does not work with 16-byte instructions and
> > bpf_patch_insn_data()? Note that e.g. 'if (insn->imm == BPF_FUNC_timer_set_callback) {'
> > case in the same bpf_do_misc_fixups() applies a patch containing BPF_LD_IMM64().
> 
> The BPF_FUNC_timer_set_callback case is different from this patch.
> 
> The BPF_FUNC_timer_set_callback case inserts a ld_imm64 insn at the
> position of the call insn.
> 
> However, this patch is going to insert a mov64_percpu_reg insn after the
> ld_imm64 insn.
> 
> Since ld_imm64 insn is a 16-byte insn, the new mov64_percpu_reg must sit
> after the ld_imm64 insn by 'i++'.
> 
> Then, let's look into bpf_patch_insn_data(). bpf_patch_insn_data()
> utilizes bpf_patch_insn_single() to insert insns at the position of one
> 8-byte insn. See memmove(..., + off + 1, ...), the *1* is for one 8-byte
> insn. I tried to modify bpf_patch_insn_single() to add 16-byte insn
> support, but that looked much complicated against this patch.

Ok, I see, it splits the original 16-byte instruction and moves the
second half after the last patch instruction. Thank you for explaining.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>