[PATCHv2 bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

[PATCHv2 bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

[Jiri Olsa]

When testing uprobes we the test gets GEP (Global Entry Point)
address from kallsyms, but then the function is called locally
so the uprobe is not triggered.

Fixing this by adjusting the address to LEP (Local Entry Point)
for powerpc arch plus instruction check stolen from ppc_function_entry
function pointed out and explained by Michael and Naveen.

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 40 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index a0ee87c8e1ea..9dc4e3dfbcf3 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -2,6 +2,44 @@
 #include <test_progs.h>
 #include "test_attach_probe.skel.h"
 
+#if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
+
+#define OP_RT_RA_MASK   0xffff0000UL
+#define LIS_R2          0x3c400000UL
+#define ADDIS_R2_R12    0x3c4c0000UL
+#define ADDI_R2_R2      0x38420000UL
+
+static ssize_t get_offset(ssize_t addr, ssize_t base)
+{
+	u32 *insn = (u32 *) addr;
+
+	/*
+	 * A PPC64 ABIv2 function may have a local and a global entry
+	 * point. We need to use the local entry point when patching
+	 * functions, so identify and step over the global entry point
+	 * sequence.
+	 *
+	 * The global entry point sequence is always of the form:
+	 *
+	 * addis r2,r12,XXXX
+	 * addi  r2,r2,XXXX
+	 *
+	 * A linker optimisation may convert the addis to lis:
+	 *
+	 * lis   r2,XXXX
+	 * addi  r2,r2,XXXX
+	 */
+	if ((((*insn & OP_RT_RA_MASK) == ADDIS_R2_R12) ||
+	     ((*insn & OP_RT_RA_MASK) == LIS_R2)) &&
+	    ((*(insn + 1) & OP_RT_RA_MASK) == ADDI_R2_R2))
+		return (ssize_t)(insn + 2) - base;
+	else
+		return addr - base;
+}
+#else
+#define get_offset(addr, base) (addr - base)
+#endif
+
 ssize_t get_base_addr() {
 	size_t start, offset;
 	char buf[256];
@@ -36,7 +74,7 @@ void test_attach_probe(void)
 	if (CHECK(base_addr < 0, "get_base_addr",
 		  "failed to find base addr: %zd", base_addr))
 		return;
-	uprobe_offset = (size_t)&get_base_addr - base_addr;
+	uprobe_offset = get_offset((size_t)&get_base_addr, base_addr);
 
 	skel = test_attach_probe__open_and_load();
 	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
-- 
2.27.0

Re: [PATCHv2 bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

[Andrii Nakryiko]

On Fri, Mar 5, 2021 at 5:42 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> When testing uprobes we the test gets GEP (Global Entry Point)
> address from kallsyms, but then the function is called locally
> so the uprobe is not triggered.
>
> Fixing this by adjusting the address to LEP (Local Entry Point)
> for powerpc arch plus instruction check stolen from ppc_function_entry
> function pointed out and explained by Michael and Naveen.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../selftests/bpf/prog_tests/attach_probe.c   | 40 ++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> index a0ee87c8e1ea..9dc4e3dfbcf3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -2,6 +2,44 @@
>  #include <test_progs.h>
>  #include "test_attach_probe.skel.h"
>
> +#if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
> +
> +#define OP_RT_RA_MASK   0xffff0000UL
> +#define LIS_R2          0x3c400000UL
> +#define ADDIS_R2_R12    0x3c4c0000UL
> +#define ADDI_R2_R2      0x38420000UL
> +
> +static ssize_t get_offset(ssize_t addr, ssize_t base)
> +{
> +       u32 *insn = (u32 *) addr;
> +
> +       /*
> +        * A PPC64 ABIv2 function may have a local and a global entry
> +        * point. We need to use the local entry point when patching
> +        * functions, so identify and step over the global entry point
> +        * sequence.
> +        *
> +        * The global entry point sequence is always of the form:
> +        *
> +        * addis r2,r12,XXXX
> +        * addi  r2,r2,XXXX
> +        *
> +        * A linker optimisation may convert the addis to lis:
> +        *
> +        * lis   r2,XXXX
> +        * addi  r2,r2,XXXX
> +        */
> +       if ((((*insn & OP_RT_RA_MASK) == ADDIS_R2_R12) ||
> +            ((*insn & OP_RT_RA_MASK) == LIS_R2)) &&
> +           ((*(insn + 1) & OP_RT_RA_MASK) == ADDI_R2_R2))
> +               return (ssize_t)(insn + 2) - base;
> +       else
> +               return addr - base;
> +}
> +#else
> +#define get_offset(addr, base) (addr - base)

I turned this into a static function, not sure why you preferred
#define here. Applied to bpf-next.

> +#endif
> +
>  ssize_t get_base_addr() {
>         size_t start, offset;
>         char buf[256];
> @@ -36,7 +74,7 @@ void test_attach_probe(void)
>         if (CHECK(base_addr < 0, "get_base_addr",
>                   "failed to find base addr: %zd", base_addr))
>                 return;
> -       uprobe_offset = (size_t)&get_base_addr - base_addr;
> +       uprobe_offset = get_offset((size_t)&get_base_addr, base_addr);
>
>         skel = test_attach_probe__open_and_load();
>         if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> --
> 2.27.0
>

Re: [PATCHv2 bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

[Jiri Olsa]

On Sat, Mar 06, 2021 at 07:13:17PM -0800, Andrii Nakryiko wrote:
> On Fri, Mar 5, 2021 at 5:42 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > When testing uprobes we the test gets GEP (Global Entry Point)
> > address from kallsyms, but then the function is called locally
> > so the uprobe is not triggered.
> >
> > Fixing this by adjusting the address to LEP (Local Entry Point)
> > for powerpc arch plus instruction check stolen from ppc_function_entry
> > function pointed out and explained by Michael and Naveen.
> >
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  .../selftests/bpf/prog_tests/attach_probe.c   | 40 ++++++++++++++++++-
> >  1 file changed, 39 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > index a0ee87c8e1ea..9dc4e3dfbcf3 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > @@ -2,6 +2,44 @@
> >  #include <test_progs.h>
> >  #include "test_attach_probe.skel.h"
> >
> > +#if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
> > +
> > +#define OP_RT_RA_MASK   0xffff0000UL
> > +#define LIS_R2          0x3c400000UL
> > +#define ADDIS_R2_R12    0x3c4c0000UL
> > +#define ADDI_R2_R2      0x38420000UL
> > +
> > +static ssize_t get_offset(ssize_t addr, ssize_t base)
> > +{
> > +       u32 *insn = (u32 *) addr;
> > +
> > +       /*
> > +        * A PPC64 ABIv2 function may have a local and a global entry
> > +        * point. We need to use the local entry point when patching
> > +        * functions, so identify and step over the global entry point
> > +        * sequence.
> > +        *
> > +        * The global entry point sequence is always of the form:
> > +        *
> > +        * addis r2,r12,XXXX
> > +        * addi  r2,r2,XXXX
> > +        *
> > +        * A linker optimisation may convert the addis to lis:
> > +        *
> > +        * lis   r2,XXXX
> > +        * addi  r2,r2,XXXX
> > +        */
> > +       if ((((*insn & OP_RT_RA_MASK) == ADDIS_R2_R12) ||
> > +            ((*insn & OP_RT_RA_MASK) == LIS_R2)) &&
> > +           ((*(insn + 1) & OP_RT_RA_MASK) == ADDI_R2_R2))
> > +               return (ssize_t)(insn + 2) - base;
> > +       else
> > +               return addr - base;
> > +}
> > +#else
> > +#define get_offset(addr, base) (addr - base)
> 
> I turned this into a static function, not sure why you preferred

seemed simple enough to be dealt with in preprocessor,
why bother compiler ;-)

> #define here. Applied to bpf-next.

thanks,
jirka

Re: [PATCHv2 bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

[Naveen N . Rao]

On 2021/03/05 02:40PM, Jiri Olsa wrote:
> When testing uprobes we the test gets GEP (Global Entry Point)
> address from kallsyms, but then the function is called locally
> so the uprobe is not triggered.
> 
> Fixing this by adjusting the address to LEP (Local Entry Point)
> for powerpc arch plus instruction check stolen from ppc_function_entry
> function pointed out and explained by Michael and Naveen.
> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../selftests/bpf/prog_tests/attach_probe.c   | 40 ++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)

LGTM. FWIW:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Thanks,
- Naveen