Re: [PATCH bpf-next v2 1/2] bpf: Support private stack for bpf progs

[Yonghong Song <yonghong.song@linux.dev>]

On 7/21/24 8:33 PM, Eduard Zingerman wrote:
> Hi Yonghong,
>
> In general I think that changes in this patch are logical and make sense.
> I have a suggestion regarding testing JIT related changes.
>
> We currently lack a convenient way to verify jit behaviour modulo
> runtime tests. I think we should have a capability to write tests like below:
>
>      SEC("tp")
>      __jited_x86("f:	endbr64")
>      __jited_x86("13:	movabs $0x.*,%r9")
>      __jited_x86("1d:	add    %gs:0x.*,%r9")
>      __jited_x86("26:	mov    $0x1,%edi")
>      __jited_x86("2b:	mov    %rdi,-0x8(%r9)")
>      __jited_x86("2f:	mov    -0x8(%r9),%rdi")
>      __jited_x86("33:	xor    %eax,%eax")
>      __jited_x86("35:	lock xchg %rax,-0x8(%r9)")
>      __jited_x86("3a:	lock xadd %rax,-0x8(%r9)")
>      __naked void stack_access_insns(void)
>      {
>      	asm volatile (
>      	"r1 = 1;"
>      	"*(u64 *)(r10 - 8) = r1;"
>      	"r1 = *(u64 *)(r10 - 8);"
>      	"r0 = 0;"
>      	"r0 = xchg_64(r10 - 8, r0);"
>      	"r0 = atomic_fetch_add((u64 *)(r10 - 8), r0);"
>      	"exit;"
>      	::: __clobber_all);
>      }
>
> In the following branch I explored a way to add such capability:
> https://github.com/eddyz87/bpf/tree/yhs-private-stack-plus-jit-testing
>
> Beside testing exact translation, such tests also provide good
> starting point for people trying to figure out how some jit features work.
>
> The below two commits are the gist of the feature:
> 8f9361be2fb3 ("selftests/bpf: __jited_x86 test tag to check x86 assembly after jit")
> 0156b148b5b4 ("selftests/bpf: utility function to get program disassembly after jit")
>
> For "0156b148b5b4" I opted to do a popen() call and execute bpftool process,
> an alternative would be to:
> a. either link tools/bpf/bpftool/jit_disasm.c as a part of the
>     test_progs executable;
> b. call libbfd (binutils dis-assembler) directly from the bpftool.
>
> Currently bpftool can use two dis-assemblers: libbfd and llvm library,
> depending on the build environment. For CI builds libbfd is used.
> I don't know if llvm and libbfd always produce same output for
> identical binary code. Imo, if people are Ok with adding libbfd
> dependency to test_progs, option (b) is the best. If folks on the
> mailing list agree with this, I can work on updating the patches.

I think this is a good idea in the long time.
Let me try with your patch.

>
> -------------
>
> Aside from testing I agree with Andrii regarding rbp usage,
> it seems like it should be possible to do the following in prologue:
>
>      movabs $0x...,%rsp
>      add %gs:0x...,%rsp
>      push %rbp
>
> and there would be no need to modify translation for instructions
> accessing r10, plus debugger stack unrolling logic should still work?.
> Or am I mistaken?

This may not work. The 'push %rbp' does not change %rbp value which still
the original %rbp.

>
> Thanks,
> Eduard