Re: [RFC PATCH bpf-next 4/5] selftests/bpf: Add test for USDT parse of xmm reg

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, May 12, 2022 at 12:43 AM Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> Validate that bpf_get_reg_val helper solves the motivating problem of
> this patch series: USDT args passed through xmm regs. The userspace
> portion of the test forces STAP_PROBE macro to use %xmm0 and %xmm1 regs
> to pass a float and an int, which the bpf-side successfully reads using
> BPF_USDT.
>
> In the wild I discovered a sanely-configured USDT in Fedora libpthread
> using xmm regs to pass scalar values, likely due to register pressure.
> urandom_read_lib_xmm mimics this by using -ffixed-$REG flag to mark
> r11-r14 unusable and passing many USDT args.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> ---
>  tools/testing/selftests/bpf/Makefile          |  8 ++-
>  tools/testing/selftests/bpf/prog_tests/usdt.c |  7 +++
>  .../selftests/bpf/progs/test_urandom_usdt.c   | 13 ++++
>  tools/testing/selftests/bpf/urandom_read.c    |  3 +
>  .../selftests/bpf/urandom_read_lib_xmm.c      | 62 +++++++++++++++++++
>  5 files changed, 91 insertions(+), 2 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/urandom_read_lib_xmm.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 6bbc03161544..19246e34dfe1 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -172,10 +172,14 @@ $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
>         $(call msg,LIB,,$@)
>         $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
>
> -$(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
> +$(OUTPUT)/liburandom_read_xmm.so: urandom_read_lib_xmm.c
> +       $(call msg,LIB,,$@)
> +       $(Q)$(CC) -O0 -ffixed-r11 -ffixed-r12 -ffixed-r13 -ffixed-r14 -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@

this looks very x86-specific, but we support other architectures as well

looking at sdt.h, it seems like STAP_PROBEx() macros support being
called from assembly code, I wonder if it would be better to try to
figure out how to use it from assembly and use some xmm register
directly in inline assembly? I have never done that before, but am
hopeful :)

> +
> +$(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so $(OUTPUT)/liburandom_read_xmm.so
>         $(call msg,BINARY,,$@)
>         $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^)                        \
> -                 liburandom_read.so $(LDLIBS)                                 \
> +                 liburandom_read.so liburandom_read_xmm.so $(LDLIBS)          \
>                   -Wl,-rpath=. -Wl,--build-id=sha1 -o $@
>
>  $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
> diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
> index a71f51bdc08d..f98749ac74a7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/usdt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
> @@ -385,6 +385,12 @@ static void subtest_urandom_usdt(bool auto_attach)
>                         goto cleanup;
>                 skel->links.urandlib_read_with_sema = l;
>
> +               l = bpf_program__attach_usdt(skel->progs.urandlib_xmm_reg_read,
> +                                            urand_pid, "./liburandom_read_xmm.so",
> +                                            "urandlib", "xmm_reg_read", NULL);
> +               if (!ASSERT_OK_PTR(l, "urandlib_xmm_reg_read"))
> +                       goto cleanup;
> +               skel->links.urandlib_xmm_reg_read = l;
>         }
>
>         /* trigger urandom_read USDTs */
> @@ -402,6 +408,7 @@ static void subtest_urandom_usdt(bool auto_attach)
>         ASSERT_EQ(bss->urandlib_read_with_sema_call_cnt, 1, "urandlib_w_sema_cnt");
>         ASSERT_EQ(bss->urandlib_read_with_sema_buf_sz_sum, 256, "urandlib_w_sema_sum");
>
> +       ASSERT_EQ(bss->urandlib_xmm_reg_read_buf_sz_sum, 256, "liburandom_read_xmm.so");
>  cleanup:
>         if (urand_pipe)
>                 pclose(urand_pipe);
> diff --git a/tools/testing/selftests/bpf/progs/test_urandom_usdt.c b/tools/testing/selftests/bpf/progs/test_urandom_usdt.c
> index 3539b02bd5f7..575761863eb6 100644
> --- a/tools/testing/selftests/bpf/progs/test_urandom_usdt.c
> +++ b/tools/testing/selftests/bpf/progs/test_urandom_usdt.c
> @@ -67,4 +67,17 @@ int BPF_USDT(urandlib_read_with_sema, int iter_num, int iter_cnt, int buf_sz)
>         return 0;
>  }
>
> +int urandlib_xmm_reg_read_buf_sz_sum;

nit: empty line here

> +SEC("usdt/./liburandom_read_xmm.so:urandlib:xmm_reg_read")
> +int BPF_USDT(urandlib_xmm_reg_read, int *f1, int *f2, int *f3, int a, int b,
> +                                    int c /*should be float */, int d, int e,
> +                                    int f, int g, int h, int buf_sz)
> +{
> +       if (urand_pid != (bpf_get_current_pid_tgid() >> 32))
> +               return 0;
> +
> +       __sync_fetch_and_add(&urandlib_xmm_reg_read_buf_sz_sum, buf_sz);
> +       return 0;
> +}
> +

[...]