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.
Thanks,
Leon
>
> The rest of the patch lgtm.
>
>> +
>> + i++;
>> + new_prog = bpf_patch_insn_data(env, i + delta,
>> insn_buf, cnt);
>> + if (!new_prog)
>> + return -ENOMEM;
>> +
>> + delta += cnt - 1;
>> + env->prog = prog = new_prog;
>> + insn = new_prog->insnsi + i + delta;
>> + goto next_insn;
>> + }
>> +
>> if (insn->code != (BPF_JMP | BPF_CALL))
>> goto next_insn;
>> if (insn->src_reg == BPF_PSEUDO_CALL)
>
> ...
