Re: [PATCH bpf-next v6 11/12] selftests/bpf: Add test to verify xlated insns for global percpu data

[Leon Hwang]

On 16/6/26 12:23, Alexei Starovoitov wrote:
> On Mon, Jun 15, 2026 at 9:19 PM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
>>
>> On 16/6/26 05:29, Alexei Starovoitov wrote:
>>> On Mon Jun 15, 2026 at 8:26 AM PDT, Leon Hwang wrote:
>> [...]
>>>> +    ASSERT_EQ(ld_imm64_xlated[0].code, ld_imm64_raw[0].code, "ld_imm64 opcode");
>>>> +    ASSERT_TRUE(ld_imm64_xlated[0].dst_reg == ld_imm64_raw[0].dst_reg, "ld_imm64 dst_reg");
>>>> +    /*
>>>> +     * The xlated instruction has the map ID in imm and the offset
>>>> +     * in the next instruction's imm. The raw instruction just has
>>>> +     * the offset in its imm.
>>>> +     */
>>>> +    ASSERT_EQ(ld_imm64_xlated[1].imm, ld_imm64_to_u64(ld_imm64_raw), "ld_imm64 off");
>>>> +
>>>> +    mov64_percpu_reg = BPF_MOV64_PERCPU_REG(ld_imm64_raw[0].dst_reg, ld_imm64_raw[0].dst_reg);
>>>> +    ASSERT_MEMEQ(&insns[idx + 2], &mov64_percpu_reg, insn_sz, "mov64_percpu_reg");
>>>
>>> If the point of the test was to check that percpu_array_map_direct_value_meta()
>>> computes 'off' correctly then it failed to achieve that goal.
>>
>> It was to check the 'off' and the mov64_percpu_reg insn.
>>
>> To avoid relying on the insns loaded from ELF obj, use raw insns to
>> check the 'off' with a percpu_array map:
>>
>>         struct bpf_insn raw_insns[] = {
>>                 BPF_LD_MAP_VALUE(BPF_REG_1, 0, 0),
>>                 BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
>>                 BPF_EXIT_INSN(),
>>         };
>>
>>         skel = test_global_percpu_data__open_and_load();
>>         exp_off = offsetof(struct test_global_percpu_data__percpu,
>> struct_data.nums[6]);
>>         raw_insns[0].imm = bpf_map__fd(skel->maps.percpu);
>>         raw_insns[1].imm = exp_off;
>>
>>         prog_fd = bpf_prog_load(...);
>>         err = get_xlated_program(prog_fd, &insns, &cnt);
>>         idx = find_percpu_ld_imm64(insns, cnt);
>>
>>         ASSERT_EQ(insns[idx + 1].imm, exp_off, "exp_off");
>>
>> Would this achieve that goal?
> 
> Hmm. There is nothing like that in the patch.
> What are you talking about?
> Some future improvement to the patch?

It is future improvement to the patch. Sorry, it is too far from the patch.

> My question was about the patch you posted.
> What was the point of it?

The point of the patch was to check that
percpu_array_map_direct_value_meta() computes 'off' correctly against
the 'off' encoded in ELF obj, and a mov64_percpu_reg insn should be
added after loading the prog.

Thanks,
Leon